diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index a77d931689..84764c1426 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -1,930 +1,935 @@ auditReasonMap[$phid])) { $this->auditReasonMap[$phid] = array(); } $this->auditReasonMap[$phid][] = $reason; return $this; } private function getAuditReasons($phid) { if (isset($this->auditReasonMap[$phid])) { return $this->auditReasonMap[$phid]; } if ($this->getIsHeraldEditor()) { $name = 'herald'; } else { $name = $this->getActor()->getUsername(); } return array('Added by '.$name.'.'); } public function setRawPatch($patch) { $this->rawPatch = $patch; return $this; } public function getRawPatch() { return $this->rawPatch; } public function getEditorApplicationClass() { return 'PhabricatorAuditApplication'; } public function getEditorObjectsDescription() { return pht('Audits'); } public function getTransactionTypes() { $types = parent::getTransactionTypes(); $types[] = PhabricatorTransactions::TYPE_COMMENT; $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorAuditTransaction::TYPE_COMMIT; // TODO: These will get modernized eventually, but that can happen one // at a time later on. $types[] = PhabricatorAuditActionConstants::ACTION; $types[] = PhabricatorAuditActionConstants::INLINE; $types[] = PhabricatorAuditActionConstants::ADD_AUDITORS; return $types; } protected function transactionHasEffect( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorAuditActionConstants::INLINE: return $xaction->hasComment(); } return parent::transactionHasEffect($object, $xaction); } protected function getCustomTransactionOldValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorAuditActionConstants::ACTION: case PhabricatorAuditActionConstants::INLINE: case PhabricatorAuditTransaction::TYPE_COMMIT: return null; case PhabricatorAuditActionConstants::ADD_AUDITORS: // TODO: For now, just record the added PHIDs. Eventually, turn these // into real edge transactions, probably? return array(); } return parent::getCustomTransactionOldValue($object, $xaction); } protected function getCustomTransactionNewValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorAuditActionConstants::ACTION: case PhabricatorAuditActionConstants::INLINE: case PhabricatorAuditActionConstants::ADD_AUDITORS: case PhabricatorAuditTransaction::TYPE_COMMIT: return $xaction->getNewValue(); } return parent::getCustomTransactionNewValue($object, $xaction); } protected function applyCustomInternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_COMMENT: case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_EDGE: case PhabricatorAuditActionConstants::ACTION: case PhabricatorAuditActionConstants::INLINE: case PhabricatorAuditActionConstants::ADD_AUDITORS: case PhabricatorAuditTransaction::TYPE_COMMIT: return; } return parent::applyCustomInternalTransaction($object, $xaction); } protected function applyCustomExternalTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_COMMENT: case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_EDGE: case PhabricatorAuditActionConstants::ACTION: - case PhabricatorAuditActionConstants::INLINE: case PhabricatorAuditTransaction::TYPE_COMMIT: return; + case PhabricatorAuditActionConstants::INLINE: + $reply = $xaction->getComment()->getReplyToComment(); + if ($reply && !$reply->getHasReplies()) { + $reply->setHasReplies(1)->save(); + } + return; case PhabricatorAuditActionConstants::ADD_AUDITORS: $new = $xaction->getNewValue(); if (!is_array($new)) { $new = array(); } $old = $xaction->getOldValue(); if (!is_array($old)) { $old = array(); } $add = array_diff_key($new, $old); $actor = $this->requireActor(); $requests = $object->getAudits(); $requests = mpull($requests, null, 'getAuditorPHID'); foreach ($add as $phid) { if (isset($requests[$phid])) { continue; } if ($this->getIsHeraldEditor()) { $audit_requested = $xaction->getMetadataValue('auditStatus'); $audit_reason_map = $xaction->getMetadataValue('auditReasonMap'); $audit_reason = $audit_reason_map[$phid]; } else { $audit_requested = PhabricatorAuditStatusConstants::AUDIT_REQUESTED; $audit_reason = $this->getAuditReasons($phid); } $requests[] = id (new PhabricatorRepositoryAuditRequest()) ->setCommitPHID($object->getPHID()) ->setAuditorPHID($phid) ->setAuditStatus($audit_requested) ->setAuditReasons($audit_reason) ->save(); } $object->attachAudits($requests); return; } return parent::applyCustomExternalTransaction($object, $xaction); } protected function applyFinalEffects( PhabricatorLiskDAO $object, array $xactions) { // Load auditors explicitly; we may not have them if the caller was a // generic piece of infrastructure. $commit = id(new DiffusionCommitQuery()) ->setViewer($this->requireActor()) ->withIDs(array($object->getID())) ->needAuditRequests(true) ->executeOne(); if (!$commit) { throw new Exception( pht('Failed to load commit during transaction finalization!')); } $object->attachAudits($commit->getAudits()); $status_concerned = PhabricatorAuditStatusConstants::CONCERNED; $status_closed = PhabricatorAuditStatusConstants::CLOSED; $status_resigned = PhabricatorAuditStatusConstants::RESIGNED; $status_accepted = PhabricatorAuditStatusConstants::ACCEPTED; $status_concerned = PhabricatorAuditStatusConstants::CONCERNED; $actor_phid = $this->getActingAsPHID(); $actor_is_author = ($object->getAuthorPHID()) && ($actor_phid == $object->getAuthorPHID()); $import_status_flag = null; foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorAuditTransaction::TYPE_COMMIT: $import_status_flag = PhabricatorRepositoryCommit::IMPORTED_HERALD; break; case PhabricatorAuditActionConstants::ACTION: $new = $xaction->getNewValue(); switch ($new) { case PhabricatorAuditActionConstants::CLOSE: // "Close" means wipe out all the concerns. $requests = $object->getAudits(); foreach ($requests as $request) { if ($request->getAuditStatus() == $status_concerned) { $request ->setAuditStatus($status_closed) ->save(); } } break; case PhabricatorAuditActionConstants::RESIGN: $requests = $object->getAudits(); $requests = mpull($requests, null, 'getAuditorPHID'); $actor_request = idx($requests, $actor_phid); // If the actor doesn't currently have a relationship to the // commit, add one explicitly. For example, this allows members // of a project to resign from a commit and have it drop out of // their queue. if (!$actor_request) { $actor_request = id(new PhabricatorRepositoryAuditRequest()) ->setCommitPHID($object->getPHID()) ->setAuditorPHID($actor_phid); $requests[] = $actor_request; $object->attachAudits($requests); } $actor_request ->setAuditStatus($status_resigned) ->save(); break; case PhabricatorAuditActionConstants::ACCEPT: case PhabricatorAuditActionConstants::CONCERN: if ($new == PhabricatorAuditActionConstants::ACCEPT) { $new_status = $status_accepted; } else { $new_status = $status_concerned; } $requests = $object->getAudits(); $requests = mpull($requests, null, 'getAuditorPHID'); // Figure out which requests the actor has authority over: these // are user requests where they are the auditor, and packages // and projects they are a member of. if ($actor_is_author) { // When modifying your own commits, you act only on behalf of // yourself, not your packages/projects -- the idea being that // you can't accept your own commits. $authority_phids = array($actor_phid); } else { $authority_phids = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser( $this->requireActor()); } $authority = array_select_keys( $requests, $authority_phids); if (!$authority) { // If the actor has no authority over any existing requests, // create a new request for them. $actor_request = id(new PhabricatorRepositoryAuditRequest()) ->setCommitPHID($object->getPHID()) ->setAuditorPHID($actor_phid) ->setAuditStatus($new_status) ->save(); $requests[$actor_phid] = $actor_request; $object->attachAudits($requests); } else { // Otherwise, update the audit status of the existing requests. foreach ($authority as $request) { $request ->setAuditStatus($new_status) ->save(); } } break; } break; } } $requests = $object->getAudits(); $object->updateAuditStatus($requests); $object->save(); if ($import_status_flag) { $object->writeImportStatusFlag($import_status_flag); } return $xactions; } protected function expandTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $xactions = parent::expandTransaction($object, $xaction); switch ($xaction->getTransactionType()) { case PhabricatorAuditTransaction::TYPE_COMMIT: $request = $this->createAuditRequestTransactionFromCommitMessage( $object); if ($request) { $xactions[] = $request; $this->setUnmentionablePHIDMap($request->getNewValue()); } break; default: break; } return $xactions; } private function createAuditRequestTransactionFromCommitMessage( PhabricatorRepositoryCommit $commit) { $data = $commit->getCommitData(); $message = $data->getCommitMessage(); $matches = null; if (!preg_match('/^Auditors:\s*(.*)$/im', $message, $matches)) { return array(); } $phids = id(new PhabricatorObjectListQuery()) ->setViewer($this->getActor()) ->setAllowPartialResults(true) ->setAllowedTypes( array( PhabricatorPeopleUserPHIDType::TYPECONST, PhabricatorProjectProjectPHIDType::TYPECONST, )) ->setObjectList($matches[1]) ->execute(); if (!$phids) { return array(); } foreach ($phids as $phid) { $this->addAuditReason($phid, 'Requested by Author'); } return id(new PhabricatorAuditTransaction()) ->setTransactionType(PhabricatorAuditActionConstants::ADD_AUDITORS) ->setNewValue(array_fuse($phids)); } protected function sortTransactions(array $xactions) { $xactions = parent::sortTransactions($xactions); $head = array(); $tail = array(); foreach ($xactions as $xaction) { $type = $xaction->getTransactionType(); if ($type == PhabricatorAuditActionConstants::INLINE) { $tail[] = $xaction; } else { $head[] = $xaction; } } return array_values(array_merge($head, $tail)); } protected function validateTransaction( PhabricatorLiskDAO $object, $type, array $xactions) { $errors = parent::validateTransaction($object, $type, $xactions); foreach ($xactions as $xaction) { switch ($type) { case PhabricatorAuditActionConstants::ACTION: $error = $this->validateAuditAction( $object, $type, $xaction, $xaction->getNewValue()); if ($error) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), $error, $xaction); } break; } } return $errors; } private function validateAuditAction( PhabricatorLiskDAO $object, $type, PhabricatorAuditTransaction $xaction, $action) { $can_author_close_key = 'audit.can-author-close-audit'; $can_author_close = PhabricatorEnv::getEnvConfig($can_author_close_key); $actor_is_author = ($object->getAuthorPHID()) && ($object->getAuthorPHID() == $this->getActingAsPHID()); switch ($action) { case PhabricatorAuditActionConstants::CLOSE: if (!$actor_is_author) { return pht( 'You can not close this audit because you are not the author '. 'of the commit.'); } if (!$can_author_close) { return pht( 'You can not close this audit because "%s" is disabled in '. 'the Phabricator configuration.', $can_author_close_key); } break; } return null; } protected function supportsSearch() { return true; } protected function expandCustomRemarkupBlockTransactions( PhabricatorLiskDAO $object, array $xactions, $blocks, PhutilMarkupEngine $engine) { // we are only really trying to find unmentionable phids here... // don't bother with this outside initial commit (i.e. create) // transaction $is_commit = false; foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorAuditTransaction::TYPE_COMMIT: $is_commit = true; break; } } // "result" is always an array.... $result = array(); if (!$is_commit) { return $result; } $flat_blocks = array_mergev($blocks); $huge_block = implode("\n\n", $flat_blocks); $phid_map = array(); $phid_map[] = $this->getUnmentionablePHIDMap(); $monograms = array(); $task_refs = id(new ManiphestCustomFieldStatusParser()) ->parseCorpus($huge_block); foreach ($task_refs as $match) { foreach ($match['monograms'] as $monogram) { $monograms[] = $monogram; } } $rev_refs = id(new DifferentialCustomFieldDependsOnParser()) ->parseCorpus($huge_block); foreach ($rev_refs as $match) { foreach ($match['monograms'] as $monogram) { $monograms[] = $monogram; } } $objects = id(new PhabricatorObjectQuery()) ->setViewer($this->getActor()) ->withNames($monograms) ->execute(); $phid_map[] = mpull($objects, 'getPHID', 'getPHID'); $phid_map = array_mergev($phid_map); $this->setUnmentionablePHIDMap($phid_map); return $result; } protected function buildReplyHandler(PhabricatorLiskDAO $object) { $reply_handler = PhabricatorEnv::newObjectFromConfig( 'metamta.diffusion.reply-handler'); $reply_handler->setMailReceiver($object); return $reply_handler; } protected function getMailSubjectPrefix() { return PhabricatorEnv::getEnvConfig('metamta.diffusion.subject-prefix'); } protected function getMailThreadID(PhabricatorLiskDAO $object) { // For backward compatibility, use this legacy thread ID. return 'diffusion-audit-'.$object->getPHID(); } protected function buildMailTemplate(PhabricatorLiskDAO $object) { $identifier = $object->getCommitIdentifier(); $repository = $object->getRepository(); $monogram = $repository->getMonogram(); $summary = $object->getSummary(); $name = $repository->formatCommitName($identifier); $subject = "{$name}: {$summary}"; $thread_topic = "Commit {$monogram}{$identifier}"; $template = id(new PhabricatorMetaMTAMail()) ->setSubject($subject) ->addHeader('Thread-Topic', $thread_topic); $this->attachPatch( $template, $object); return $template; } protected function getMailTo(PhabricatorLiskDAO $object) { $phids = array(); if ($this->heraldEmailPHIDs) { $phids = $this->heraldEmailPHIDs; } if ($object->getAuthorPHID()) { $phids[] = $object->getAuthorPHID(); } $status_resigned = PhabricatorAuditStatusConstants::RESIGNED; foreach ($object->getAudits() as $audit) { if ($audit->getAuditStatus() != $status_resigned) { $phids[] = $audit->getAuditorPHID(); } } return $phids; } protected function buildMailBody( PhabricatorLiskDAO $object, array $xactions) { $body = parent::buildMailBody($object, $xactions); $type_inline = PhabricatorAuditActionConstants::INLINE; $type_push = PhabricatorAuditTransaction::TYPE_COMMIT; $is_commit = false; $inlines = array(); foreach ($xactions as $xaction) { if ($xaction->getTransactionType() == $type_inline) { $inlines[] = $xaction; } if ($xaction->getTransactionType() == $type_push) { $is_commit = true; } } if ($inlines) { $body->addTextSection( pht('INLINE COMMENTS'), $this->renderInlineCommentsForMail($object, $inlines)); } if ($is_commit) { $data = $object->getCommitData(); $body->addTextSection(pht('AFFECTED FILES'), $this->affectedFiles); $this->inlinePatch( $body, $object); } // Reload the commit to pull commit data. $commit = id(new DiffusionCommitQuery()) ->setViewer($this->requireActor()) ->withIDs(array($object->getID())) ->needCommitData(true) ->executeOne(); $data = $commit->getCommitData(); $user_phids = array(); $author_phid = $commit->getAuthorPHID(); if ($author_phid) { $user_phids[$author_phid][] = pht('Author'); } $committer_phid = $data->getCommitDetail('committerPHID'); if ($committer_phid && ($committer_phid != $author_phid)) { $user_phids[$committer_phid][] = pht('Committer'); } // we loaded this in applyFinalEffects $audit_requests = $object->getAudits(); $auditor_phids = mpull($audit_requests, 'getAuditorPHID'); foreach ($auditor_phids as $auditor_phid) { $user_phids[$auditor_phid][] = pht('Auditor'); } // TODO: It would be nice to show pusher here too, but that information // is a little tricky to get at right now. if ($user_phids) { $handle_phids = array_keys($user_phids); $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->requireActor()) ->withPHIDs($handle_phids) ->execute(); $user_info = array(); foreach ($user_phids as $phid => $roles) { $user_info[] = pht( '%s (%s)', $handles[$phid]->getName(), implode(', ', $roles)); } $body->addTextSection( pht('USERS'), implode("\n", $user_info)); } $monogram = $object->getRepository()->formatCommitName( $object->getCommitIdentifier()); $body->addLinkSection( pht('COMMIT'), PhabricatorEnv::getProductionURI('/'.$monogram)); return $body; } private function attachPatch( PhabricatorMetaMTAMail $template, PhabricatorRepositoryCommit $commit) { if (!$this->getRawPatch()) { return; } $attach_key = 'metamta.diffusion.attach-patches'; $attach_patches = PhabricatorEnv::getEnvConfig($attach_key); if (!$attach_patches) { return; } $repository = $commit->getRepository(); $encoding = $repository->getDetail('encoding', 'UTF-8'); $raw_patch = $this->getRawPatch(); $commit_name = $repository->formatCommitName( $commit->getCommitIdentifier()); $template->addAttachment( new PhabricatorMetaMTAAttachment( $raw_patch, $commit_name.'.patch', 'text/x-patch; charset='.$encoding)); } private function inlinePatch( PhabricatorMetaMTAMailBody $body, PhabricatorRepositoryCommit $commit) { if (!$this->getRawPatch()) { return; } $inline_key = 'metamta.diffusion.inline-patches'; $inline_patches = PhabricatorEnv::getEnvConfig($inline_key); if (!$inline_patches) { return; } $repository = $commit->getRepository(); $raw_patch = $this->getRawPatch(); $result = null; $len = substr_count($raw_patch, "\n"); if ($len <= $inline_patches) { // We send email as utf8, so we need to convert the text to utf8 if // we can. $encoding = $repository->getDetail('encoding', 'UTF-8'); if ($encoding) { $raw_patch = phutil_utf8_convert($raw_patch, 'UTF-8', $encoding); } $result = phutil_utf8ize($raw_patch); } if ($result) { $result = "PATCH\n\n{$result}\n"; } $body->addRawSection($result); } private function renderInlineCommentsForMail( PhabricatorLiskDAO $object, array $inline_xactions) { $inlines = mpull($inline_xactions, 'getComment'); $block = array(); $path_map = id(new DiffusionPathQuery()) ->withPathIDs(mpull($inlines, 'getPathID')) ->execute(); $path_map = ipull($path_map, 'path', 'id'); foreach ($inlines as $inline) { $path = idx($path_map, $inline->getPathID()); if ($path === null) { continue; } $start = $inline->getLineNumber(); $len = $inline->getLineLength(); if ($len) { $range = $start.'-'.($start + $len); } else { $range = $start; } $content = $inline->getContent(); $block[] = "{$path}:{$range} {$content}"; } return implode("\n", $block); } public function getMailTagsMap() { return array( PhabricatorAuditTransaction::MAILTAG_COMMIT => pht('A commit is created.'), PhabricatorAuditTransaction::MAILTAG_ACTION_CONCERN => pht('A commit has a concerned raised against it.'), PhabricatorAuditTransaction::MAILTAG_ACTION_ACCEPT => pht('A commit is accepted.'), PhabricatorAuditTransaction::MAILTAG_ACTION_RESIGN => pht('A commit has an auditor resign.'), PhabricatorAuditTransaction::MAILTAG_ACTION_CLOSE => pht('A commit is closed.'), PhabricatorAuditTransaction::MAILTAG_ADD_AUDITORS => pht('A commit has auditors added.'), PhabricatorAuditTransaction::MAILTAG_ADD_CCS => pht("A commit's subscribers change."), PhabricatorAuditTransaction::MAILTAG_PROJECTS => pht("A commit's projects change."), PhabricatorAuditTransaction::MAILTAG_COMMENT => pht('Someone comments on a commit.'), PhabricatorAuditTransaction::MAILTAG_OTHER => pht('Other commit activity not listed above occurs.'), ); } protected function shouldApplyHeraldRules( PhabricatorLiskDAO $object, array $xactions) { foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case PhabricatorAuditTransaction::TYPE_COMMIT: $repository = $object->getRepository(); if (!$repository->shouldPublish()) { return false; } return true; default: break; } } return parent::shouldApplyHeraldRules($object, $xactions); } protected function buildHeraldAdapter( PhabricatorLiskDAO $object, array $xactions) { return id(new HeraldCommitAdapter()) ->setCommit($object); } protected function didApplyHeraldRules( PhabricatorLiskDAO $object, HeraldAdapter $adapter, HeraldTranscript $transcript) { $xactions = array(); $audit_phids = $adapter->getAuditMap(); foreach ($audit_phids as $phid => $rule_ids) { foreach ($rule_ids as $rule_id) { $this->addAuditReason( $phid, pht( '%s Triggered Audit', "H{$rule_id}")); } } if ($audit_phids) { $xactions[] = id(new PhabricatorAuditTransaction()) ->setTransactionType(PhabricatorAuditActionConstants::ADD_AUDITORS) ->setNewValue(array_fuse(array_keys($audit_phids))) ->setMetadataValue( 'auditStatus', PhabricatorAuditStatusConstants::AUDIT_REQUIRED) ->setMetadataValue( 'auditReasonMap', $this->auditReasonMap); } $cc_phids = $adapter->getAddCCMap(); $add_ccs = array('+' => array()); foreach ($cc_phids as $phid => $rule_ids) { $add_ccs['+'][$phid] = $phid; } $xactions[] = id(new PhabricatorAuditTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) ->setNewValue($add_ccs); $this->heraldEmailPHIDs = $adapter->getEmailPHIDs(); HarbormasterBuildable::applyBuildPlans( $object->getPHID(), $object->getRepository()->getPHID(), $adapter->getBuildPlans()); $limit = self::MAX_FILES_SHOWN_IN_EMAIL; $files = $adapter->loadAffectedPaths(); sort($files); if (count($files) > $limit) { array_splice($files, $limit); $files[] = pht( '(This commit affected more than %d files. Only %d are shown here '. 'and additional ones are truncated.)', $limit, $limit); } $this->affectedFiles = implode("\n", $files); return $xactions; } private function isCommitMostlyImported(PhabricatorLiskDAO $object) { $has_message = PhabricatorRepositoryCommit::IMPORTED_MESSAGE; $has_changes = PhabricatorRepositoryCommit::IMPORTED_CHANGE; // Don't publish feed stories or email about events which occur during // import. In particular, this affects tasks being attached when they are // closed by "Fixes Txxxx" in a commit message. See T5851. $mask = ($has_message | $has_changes); return $object->isPartiallyImported($mask); } private function shouldPublishRepositoryActivity( PhabricatorLiskDAO $object, array $xactions) { // not every code path loads the repository so tread carefully // TODO: They should, and then we should simplify this. if ($object->getRepository($assert_attached = false)) { $repository = $object->getRepository(); if (!$repository->shouldPublish()) { return false; } } return $this->isCommitMostlyImported($object); } protected function shouldSendMail( PhabricatorLiskDAO $object, array $xactions) { return $this->shouldPublishRepositoryActivity($object, $xactions); } protected function shouldEnableMentions( PhabricatorLiskDAO $object, array $xactions) { return $this->shouldPublishRepositoryActivity($object, $xactions); } protected function shouldPublishFeedStory( PhabricatorLiskDAO $object, array $xactions) { return $this->shouldPublishRepositoryActivity($object, $xactions); } } diff --git a/src/applications/audit/storage/PhabricatorAuditInlineComment.php b/src/applications/audit/storage/PhabricatorAuditInlineComment.php index c3f2263de6..13cb76174d 100644 --- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php +++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php @@ -1,312 +1,317 @@ proxy = new PhabricatorAuditTransactionComment(); } public function __clone() { $this->proxy = clone $this->proxy; } public function getTransactionPHID() { return $this->proxy->getTransactionPHID(); } public function getTransactionComment() { return $this->proxy; } public function getTransactionCommentForSave() { $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_LEGACY, array()); $this->proxy ->setViewPolicy('public') ->setEditPolicy($this->getAuthorPHID()) ->setContentSource($content_source) ->setCommentVersion(1); return $this->proxy; } public static function loadID($id) { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'id = %d', $id); if (!$inlines) { return null; } return head(self::buildProxies($inlines)); } public static function loadPHID($phid) { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'phid = %s', $phid); if (!$inlines) { return null; } return head(self::buildProxies($inlines)); } public static function loadDraftComments( PhabricatorUser $viewer, $commit_phid) { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'authorPHID = %s AND commitPHID = %s AND transactionPHID IS NULL - AND pathID IS NOT NULL', + AND pathID IS NOT NULL + AND isDeleted = 0', $viewer->getPHID(), $commit_phid); + $inlines = PhabricatorInlineCommentController::loadAndAttachReplies( + $viewer, + $inlines); + return self::buildProxies($inlines); } public static function loadPublishedComments( PhabricatorUser $viewer, $commit_phid) { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'commitPHID = %s AND transactionPHID IS NOT NULL AND pathID IS NOT NULL', $commit_phid); return self::buildProxies($inlines); } public static function loadDraftAndPublishedComments( PhabricatorUser $viewer, $commit_phid, $path_id = null) { if ($path_id === null) { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'commitPHID = %s AND (transactionPHID IS NOT NULL OR authorPHID = %s) AND pathID IS NOT NULL', $commit_phid, $viewer->getPHID()); } else { $inlines = id(new PhabricatorAuditTransactionComment())->loadAllWhere( 'commitPHID = %s AND pathID = %d AND - (authorPHID = %s OR transactionPHID IS NOT NULL)', + ((authorPHID = %s AND isDeleted = 0) OR transactionPHID IS NOT NULL)', $commit_phid, $path_id, $viewer->getPHID()); } return self::buildProxies($inlines); } private static function buildProxies(array $inlines) { $results = array(); foreach ($inlines as $key => $inline) { $results[$key] = PhabricatorAuditInlineComment::newFromModernComment( $inline); } return $results; } public function setSyntheticAuthor($synthetic_author) { $this->syntheticAuthor = $synthetic_author; return $this; } public function getSyntheticAuthor() { return $this->syntheticAuthor; } public function openTransaction() { $this->proxy->openTransaction(); } public function saveTransaction() { $this->proxy->saveTransaction(); } public function save() { $this->getTransactionCommentForSave()->save(); return $this; } public function delete() { $this->proxy->delete(); return $this; } public function getID() { return $this->proxy->getID(); } public function getPHID() { return $this->proxy->getPHID(); } public static function newFromModernComment( PhabricatorAuditTransactionComment $comment) { $obj = new PhabricatorAuditInlineComment(); $obj->proxy = $comment; return $obj; } public function isCompatible(PhabricatorInlineCommentInterface $comment) { return ($this->getAuthorPHID() === $comment->getAuthorPHID()) && ($this->getSyntheticAuthor() === $comment->getSyntheticAuthor()) && ($this->getContent() === $comment->getContent()); } public function setContent($content) { $this->proxy->setContent($content); return $this; } public function getContent() { return $this->proxy->getContent(); } public function isDraft() { return !$this->proxy->getTransactionPHID(); } public function setPathID($id) { $this->proxy->setPathID($id); return $this; } public function getPathID() { return $this->proxy->getPathID(); } public function setIsNewFile($is_new) { $this->proxy->setIsNewFile($is_new); return $this; } public function getIsNewFile() { return $this->proxy->getIsNewFile(); } public function setLineNumber($number) { $this->proxy->setLineNumber($number); return $this; } public function getLineNumber() { return $this->proxy->getLineNumber(); } public function setLineLength($length) { $this->proxy->setLineLength($length); return $this; } public function getLineLength() { return $this->proxy->getLineLength(); } public function setCache($cache) { return $this; } public function getCache() { return null; } public function setAuthorPHID($phid) { $this->proxy->setAuthorPHID($phid); return $this; } public function getAuthorPHID() { return $this->proxy->getAuthorPHID(); } public function setCommitPHID($commit_phid) { $this->proxy->setCommitPHID($commit_phid); return $this; } public function getCommitPHID() { return $this->proxy->getCommitPHID(); } // When setting a comment ID, we also generate a phantom transaction PHID for // the future transaction. public function setAuditCommentID($id) { $this->proxy->setLegacyCommentID($id); $this->proxy->setTransactionPHID( PhabricatorPHID::generateNewPHID( PhabricatorApplicationTransactionTransactionPHIDType::TYPECONST, PhabricatorRepositoryCommitPHIDType::TYPECONST)); return $this; } public function getAuditCommentID() { return $this->proxy->getLegacyCommentID(); } public function setChangesetID($id) { return $this->setPathID($id); } public function getChangesetID() { return $this->getPathID(); } public function setReplyToCommentPHID($phid) { $this->proxy->setReplyToCommentPHID($phid); return $this; } public function getReplyToCommentPHID() { return $this->proxy->getReplyToCommentPHID(); } public function setHasReplies($has_replies) { $this->proxy->setHasReplies($has_replies); return $this; } public function getHasReplies() { return $this->proxy->getHasReplies(); } /* -( PhabricatorMarkupInterface Implementation )-------------------------- */ public function getMarkupFieldKey($field) { return 'AI:'.$this->getID(); } public function newMarkupEngine($field) { return PhabricatorMarkupEngine::newDifferentialMarkupEngine(); } public function getMarkupText($field) { return $this->getContent(); } public function didMarkupText($field, $output, PhutilMarkupEngine $engine) { return $output; } public function shouldUseMarkupCache($field) { // Only cache submitted comments. return ($this->getID() && $this->getAuditCommentID()); } } diff --git a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php index 9a9925258e..64ddca8e25 100644 --- a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php +++ b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php @@ -1,58 +1,70 @@ getTransactionPHID() != null); } protected function getConfiguration() { $config = parent::getConfiguration(); $config[self::CONFIG_COLUMN_SCHEMA] = array( 'commitPHID' => 'phid?', 'pathID' => 'id?', 'isNewFile' => 'bool', 'lineNumber' => 'uint32', 'lineLength' => 'uint32', 'fixedState' => 'text12?', 'hasReplies' => 'bool', 'replyToCommentPHID' => 'phid?', 'legacyCommentID' => 'id?', ) + $config[self::CONFIG_COLUMN_SCHEMA]; $config[self::CONFIG_KEY_SCHEMA] = array( 'key_path' => array( 'columns' => array('pathID'), ), 'key_draft' => array( 'columns' => array('authorPHID', 'transactionPHID'), ), 'key_commit' => array( 'columns' => array('commitPHID'), ), 'key_legacy' => array( 'columns' => array('legacyCommentID'), ), ) + $config[self::CONFIG_KEY_SCHEMA]; return $config; } + public function attachReplyToComment( + PhabricatorAuditTransactionComment $comment = null) { + $this->replyToComment = $comment; + return $this; + } + + public function getReplyToComment() { + return $this->assertAttached($this->replyToComment); + } + } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index e17746fce0..c3dd3c3ace 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1,1867 +1,1872 @@ getTransactionType() == $type_update) { return $xaction; } } return null; } public function setIsCloseByCommit($is_close_by_commit) { $this->isCloseByCommit = $is_close_by_commit; return $this; } public function getIsCloseByCommit() { return $this->isCloseByCommit; } public function setChangedPriorToCommitURI($uri) { $this->changedPriorToCommitURI = $uri; return $this; } public function getChangedPriorToCommitURI() { return $this->changedPriorToCommitURI; } public function setRepositoryPHIDOverride($phid_or_null) { $this->repositoryPHIDOverride = $phid_or_null; return $this; } public function getTransactionTypes() { $types = parent::getTransactionTypes(); $types[] = PhabricatorTransactions::TYPE_COMMENT; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = DifferentialTransaction::TYPE_ACTION; $types[] = DifferentialTransaction::TYPE_INLINE; $types[] = DifferentialTransaction::TYPE_STATUS; $types[] = DifferentialTransaction::TYPE_UPDATE; return $types; } protected function getCustomTransactionOldValue( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->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) { $actor_phid = $this->getActingAsPHID(); 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(); // 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_plan); case DifferentialAction::ACTION_REQUEST: return ($object->getStatus() != $status_review); case DifferentialAction::ACTION_RESIGN: foreach ($object->getReviewerStatus() as $reviewer) { if ($reviewer->getReviewerPHID() == $actor_phid) { return true; } } return false; case DifferentialAction::ACTION_CLAIM: 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: if (!$this->getIsCloseByCommit() && (($object->getStatus() == $status_revision) || ($object->getStatus() == $status_plan))) { $object->setStatus($status_review); } $diff = $this->requireDiff($xaction->getNewValue()); $object->setLineCount($diff->getLineCount()); if ($this->repositoryPHIDOverride !== false) { $object->setRepositoryPHID($this->repositoryPHIDOverride); } else { $object->setRepositoryPHID($diff->getRepositoryPHID()); } $object->setArcanistProjectPHID($diff->getArcanistProjectPHID()); $object->attachActiveDiff($diff); // 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_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->getActingAsPHID()); return; } break; } return parent::applyCustomInternalTransaction($object, $xaction); } protected function expandTransaction( PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { $results = parent::expandTransaction($object, $xaction); $actor = $this->getActor(); $actor_phid = $this->getActingAsPHID(); $type_edge = PhabricatorTransactions::TYPE_EDGE; $status_plan = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED; $edge_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST; $edge_ref_task = DifferentialRevisionHasTaskEdgeType::EDGECONST; $is_sticky_accept = PhabricatorEnv::getEnvConfig( 'differential.sticky-accept'); $downgrade_rejects = false; $downgrade_accepts = false; if ($this->getIsCloseByCommit()) { // Never downgrade reviewers when we're closing a revision after a // commit. } else { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $downgrade_rejects = true; if (!$is_sticky_accept) { // If "sticky accept" is disabled, also downgrade the accepts. $downgrade_accepts = true; } break; case DifferentialTransaction::TYPE_ACTION: switch ($xaction->getNewValue()) { case DifferentialAction::ACTION_REQUEST: $downgrade_rejects = true; if ((!$is_sticky_accept) || ($object->getStatus() != $status_plan)) { // If the old state isn't "changes planned", downgrade the // accepts. This exception allows an accepted revision to // go through Plan Changes -> Request Review to return to // "accepted" if the author didn't update the revision. $downgrade_accepts = true; } break; } break; } } $new_accept = DifferentialReviewerStatus::STATUS_ACCEPTED; $new_reject = DifferentialReviewerStatus::STATUS_REJECTED; $old_accept = DifferentialReviewerStatus::STATUS_ACCEPTED_OLDER; $old_reject = DifferentialReviewerStatus::STATUS_REJECTED_OLDER; if ($downgrade_rejects || $downgrade_accepts) { // 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". // We also do this for "Request Review", even though the diff is not // updated directly. Essentially, this acts like an update which doesn't // actually change the diff text. $edits = array(); foreach ($object->getReviewerStatus() as $reviewer) { if ($downgrade_rejects) { if ($reviewer->getStatus() == $new_reject) { $edits[$reviewer->getReviewerPHID()] = array( 'data' => array( 'status' => $old_reject, ), ); } } if ($downgrade_accepts) { if ($reviewer->getStatus() == $new_accept) { $edits[$reviewer->getReviewerPHID()] = array( 'data' => array( 'status' => $old_accept, ), ); } } } if ($edits) { $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => $edits)); } } switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: if ($this->getIsCloseByCommit()) { // Don't bother with any of this if this update is a side effect of // commit detection. break; } // When a revision is updated and the diff comes from a branch named // "T123" or similar, automatically associate the commit with the // task that the branch names. $maniphest = 'PhabricatorManiphestApplication'; if (PhabricatorApplication::isClassInstalled($maniphest)) { $diff = $this->requireDiff($xaction->getNewValue()); $branch = $diff->getBranch(); // No "$", to allow for branches like T123_demo. $match = null; if (preg_match('/^T(\d+)/i', $branch, $match)) { $task_id = $match[1]; $tasks = id(new ManiphestTaskQuery()) ->setViewer($this->getActor()) ->withIDs(array($task_id)) ->execute(); if ($tasks) { $task = head($tasks); $task_phid = $task->getPHID(); $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_ref_task) ->setIgnoreOnNoEffect(true) ->setNewValue(array('+' => array($task_phid => $task_phid))); } } } 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(), ), ); } // NOTE: We're setting setIsCommandeerSideEffect() on this because // normally you can't add a revision's author as a reviewer, but // this action swaps them after validation executes. $results[] = id(new DifferentialTransaction()) ->setTransactionType($type_edge) ->setMetadataValue('edge:type', $edge_reviewer) ->setIgnoreOnNoEffect(true) ->setIsCommandeerSideEffect(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: + return; case DifferentialTransaction::TYPE_INLINE: + $reply = $xaction->getComment()->getReplyToComment(); + if ($reply && !$reply->getHasReplies()) { + $reply->setHasReplies(1)->save(); + } return; case DifferentialTransaction::TYPE_UPDATE: // Now that we're inside the transaction, do a final check. $diff = $this->requireDiff($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. $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(); return; } return parent::applyCustomExternalTransaction($object, $xaction); } protected function mergeEdgeData($type, array $u, array $v) { $result = parent::mergeEdgeData($type, $u, $v); switch ($type) { case DifferentialRevisionHasReviewerEdgeType::EDGECONST: // 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) { // 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. Then, update the reviewers // on the object to make sure we're acting on the current reviewer set // (and, for example, sending mail to the right people). $new_revision = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->needReviewerStatus(true) ->needActiveDiffs(true) ->withIDs(array($object->getID())) ->executeOne(); if (!$new_revision) { throw new Exception( pht('Failed to load revision from transaction finalization.')); } $object->attachReviewerStatus($new_revision->getReviewerStatus()); $object->attachActiveDiff($new_revision->getActiveDiff()); $object->attachRepository($new_revision->getRepository()); foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $diff = $this->requireDiff($xaction->getNewValue(), true); // Update these denormalized index tables when we attach a new // diff to a revision. $this->updateRevisionHashTable($object, $diff); $this->updateAffectedPathTable($object, $diff); break; } } $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: // 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 ($object->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); $config_self_accept_key = 'differential.allow-self-accept'; $allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key); foreach ($xactions as $xaction) { switch ($type) { case PhabricatorTransactions::TYPE_EDGE: switch ($xaction->getMetadataValue('edge:type')) { case DifferentialRevisionHasReviewerEdgeType::EDGECONST: // Prevent the author from becoming a reviewer. // NOTE: This is pretty gross, but this restriction is unusual. // If we end up with too much more of this, we should try to clean // this up -- maybe by moving validation to after transactions // are adjusted (so we can just examine the final value) or adding // a second phase there? $author_phid = $object->getAuthorPHID(); $new = $xaction->getNewValue(); $add = idx($new, '+', array()); $eq = idx($new, '=', array()); $phids = array_keys($add + $eq); foreach ($phids as $phid) { if (($phid == $author_phid) && !$allow_self_accept && !$xaction->getIsCommandeerSideEffect()) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid'), pht('The author of a revision can not be a reviewer.'), $xaction); } } break; } break; 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->getActingAsPHID(); $actor_is_author = ($author_phid == $actor_phid); $config_abandon_key = 'differential.always-allow-abandon'; $always_allow_abandon = PhabricatorEnv::getEnvConfig($config_abandon_key); $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.'); } // TODO: It would be nice to make this generic at some point. $signatures = DifferentialRequiredSignaturesField::loadForRevision( $revision); foreach ($signatures as $phid => $signed) { if (!$signed) { return pht( 'You can not accept this revision because the author has '. 'not signed all of the required legal documents.'); } } 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 && !$always_allow_abandon) { 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 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: // We force revisions closed when we discover a corresponding commit. // In this case, revisions are allowed to transition to closed from // any state. This is an automated action taken by the daemons. if (!$this->getIsCloseByCommit()) { 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 getMailCC(PhabricatorLiskDAO $object) { $phids = parent::getMailCC($object); if ($this->heraldEmailPHIDs) { foreach ($this->heraldEmailPHIDs as $phid) { $phids[] = $phid; } } return $phids; } protected function getMailAction( PhabricatorLiskDAO $object, array $xactions) { $action = parent::getMailAction($object, $xactions); $strongest = $this->getStrongestAction($object, $xactions); switch ($strongest->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $count = new PhutilNumber($object->getLineCount()); $action = pht('%s, %s line(s)', $action, $count); break; } return $action; } 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 = new PhabricatorMetaMTAMailBody(); $body->setViewer($this->requireActor()); $this->addHeadersAndCommentsToMailBody($body, $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)); } $changed_uri = $this->getChangedPriorToCommitURI(); if ($changed_uri) { $body->addLinkSection( pht('CHANGED PRIOR TO COMMIT'), $changed_uri); } $this->addCustomFieldsToMailBody($body, $object, $xactions); $body->addLinkSection( pht('REVISION DETAIL'), PhabricatorEnv::getProductionURI('/D'.$object->getID())); $update_xaction = null; foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: $update_xaction = $xaction; break; } } if ($update_xaction) { $diff = $this->requireDiff($update_xaction->getNewValue(), true); $body->addTextSection( pht('AFFECTED FILES'), $this->renderAffectedFilesForMail($diff)); $config_key_inline = 'metamta.differential.inline-patches'; $config_inline = PhabricatorEnv::getEnvConfig($config_key_inline); $config_key_attach = 'metamta.differential.attach-patches'; $config_attach = PhabricatorEnv::getEnvConfig($config_key_attach); if ($config_inline || $config_attach) { $patch_section = $this->renderPatchForMail($diff); $lines = count(phutil_split_lines($patch_section->getPlaintext())); if ($config_inline && ($lines <= $config_inline)) { $body->addTextSection( pht('CHANGE DETAILS'), $patch_section); } if ($config_attach) { $name = pht('D%s.%s.patch', $object->getID(), $diff->getID()); $mime_type = 'text/x-patch; charset=utf-8'; $body->addAttachment( new PhabricatorMetaMTAAttachment( $patch_section->getPlaintext(), $name, $mime_type)); } } } return $body; } public function getMailTagsMap() { return array( MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEW_REQUEST => pht('A revision is created.'), MetaMTANotificationType::TYPE_DIFFERENTIAL_UPDATED => pht('A revision is updated.'), MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMENT => pht('Someone comments on a revision.'), MetaMTANotificationType::TYPE_DIFFERENTIAL_CLOSED => pht('A revision is closed.'), MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEWERS => pht("A revision's reviewers change."), MetaMTANotificationType::TYPE_DIFFERENTIAL_CC => pht("A revision's CCs change."), MetaMTANotificationType::TYPE_DIFFERENTIAL_OTHER => pht('Other revision activity not listed above occurs.'), ); } 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(); $task_phids = array(); $rev_phids = array(); if ($task_map) { $tasks = id(new ManiphestTaskQuery()) ->setViewer($this->getActor()) ->withIDs(array_keys($task_map)) ->execute(); if ($tasks) { $task_phids = mpull($tasks, 'getPHID', 'getPHID'); $edge_related = DifferentialRevisionHasTaskEdgeType::EDGECONST; $edges[$edge_related] = $task_phids; } } 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) { $depends = DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST; $edges[$depends] = $rev_phids; } } $this->setUnmentionablePHIDMap(array_merge($task_phids, $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; } protected function indentForMail(array $lines) { $indented = array(); foreach ($lines as $line) { $indented[] = '> '.$line; } return $indented; } protected function nestCommentHistory( DifferentialTransactionComment $comment, array $comments_by_line_number, array $users_by_phid) { $nested = array(); $previous_comments = $comments_by_line_number[$comment->getChangesetID()] [$comment->getLineNumber()]; foreach ($previous_comments as $previous_comment) { if ($previous_comment->getID() >= $comment->getID()) { break; } $nested = $this->indentForMail( array_merge( $nested, explode("\n", $previous_comment->getContent()))); $user = idx($users_by_phid, $previous_comment->getAuthorPHID(), null); if ($user) { array_unshift($nested, pht('%s wrote:', $user->getUserName())); } } $nested = array_merge($nested, explode("\n", $comment->getContent())); return implode("\n", $nested); } private function renderInlineCommentsForMail( PhabricatorLiskDAO $object, array $inlines) { $context_key = 'metamta.differential.unified-comment-context'; $show_context = PhabricatorEnv::getEnvConfig($context_key); $changeset_ids = array(); $line_numbers_by_changeset = array(); foreach ($inlines as $inline) { $id = $inline->getComment()->getChangesetID(); $changeset_ids[$id] = $id; $line_numbers_by_changeset[$id][] = $inline->getComment()->getLineNumber(); } $changesets = id(new DifferentialChangesetQuery()) ->setViewer($this->getActor()) ->withIDs($changeset_ids) ->needHunks(true) ->execute(); $inline_groups = DifferentialTransactionComment::sortAndGroupInlines( $inlines, $changesets); if ($show_context) { $hunk_parser = new DifferentialHunkParser(); $table = new DifferentialTransactionComment(); $conn_r = $table->establishConnection('r'); $queries = array(); foreach ($line_numbers_by_changeset as $id => $line_numbers) { $queries[] = qsprintf( $conn_r, '(changesetID = %d AND lineNumber IN (%Ld))', $id, $line_numbers); } $all_comments = id(new DifferentialTransactionComment())->loadAllWhere( 'transactionPHID IS NOT NULL AND (%Q)', implode(' OR ', $queries)); $comments_by_line_number = array(); foreach ($all_comments as $comment) { $comments_by_line_number [$comment->getChangesetID()] [$comment->getLineNumber()] [$comment->getID()] = $comment; } $author_phids = mpull($all_comments, 'getAuthorPHID'); $authors = id(new PhabricatorPeopleQuery()) ->setViewer($this->getActor()) ->withPHIDs($author_phids) ->execute(); $authors_by_phid = mpull($authors, null, 'getPHID'); } $section = new PhabricatorMetaMTAMailSection(); 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) { $section->addFragment("{$file}:{$range} {$inline_content}"); } else { $patch = $hunk_parser->makeContextDiff( $changeset->getHunks(), $comment->getIsNewFile(), $comment->getLineNumber(), $comment->getLineLength(), 1); $nested_comments = $this->nestCommentHistory( $inline->getComment(), $comments_by_line_number, $authors_by_phid); $section->addFragment('================') ->addFragment('Comment at: '.$file.':'.$range) ->addPlaintextFragment($patch) ->addHTMLFragment($this->renderPatchHTMLForMail($patch)) ->addFragment('----------------') ->addFragment($nested_comments) ->addFragment(null); } } } return $section; } private function loadDiff($phid, $need_changesets = false) { $query = id(new DifferentialDiffQuery()) ->withPHIDs(array($phid)) ->setViewer($this->getActor()); if ($need_changesets) { $query->needChangesets(true); } return $query->executeOne(); } private function requireDiff($phid, $need_changesets = false) { $diff = $this->loadDiff($phid, $need_changesets); if (!$diff) { throw new Exception(pht('Diff "%s" does not exist!', $phid)); } return $diff; } /* -( Herald Integration )------------------------------------------------- */ protected function shouldApplyHeraldRules( PhabricatorLiskDAO $object, array $xactions) { if ($this->getIsNewObject()) { return true; } foreach ($xactions as $xaction) { switch ($xaction->getTransactionType()) { case DifferentialTransaction::TYPE_UPDATE: if (!$this->getIsCloseByCommit()) { return true; } break; case DifferentialTransaction::TYPE_ACTION: switch ($xaction->getNewValue()) { case DifferentialAction::ACTION_CLAIM: // When users commandeer revisions, we may need to trigger // signatures or author-based rules. return true; } break; } } return parent::shouldApplyHeraldRules($object, $xactions); } protected function buildHeraldAdapter( PhabricatorLiskDAO $object, array $xactions) { $unsubscribed_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( $object->getPHID(), PhabricatorObjectHasUnsubscriberEdgeType::EDGECONST); $subscribed_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID( $object->getPHID()); $revision = id(new DifferentialRevisionQuery()) ->setViewer($this->getActor()) ->withPHIDs(array($object->getPHID())) ->needActiveDiffs(true) ->needReviewerStatus(true) ->executeOne(); if (!$revision) { throw new Exception( pht( 'Failed to load revision for Herald adapter construction!')); } $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( $revision, $revision->getActiveDiff()); $reviewers = $revision->getReviewerStatus(); $reviewer_phids = mpull($reviewers, 'getReviewerPHID'); $adapter->setExplicitCCs($subscribed_phids); $adapter->setExplicitReviewers($reviewer_phids); $adapter->setForbiddenCCs($unsubscribed_phids); return $adapter; } protected function didApplyHeraldRules( PhabricatorLiskDAO $object, HeraldAdapter $adapter, HeraldTranscript $transcript) { $xactions = array(); // Build a transaction to adjust CCs. $ccs = array( '+' => array_keys($adapter->getCCsAddedByHerald()), '-' => array_keys($adapter->getCCsRemovedByHerald()), ); $value = array(); foreach ($ccs as $type => $phids) { foreach ($phids as $phid) { $value[$type][$phid] = $phid; } } if ($value) { $xactions[] = id(new DifferentialTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) ->setNewValue($value); } // Build a transaction to adjust reviewers. $reviewers = array( DifferentialReviewerStatus::STATUS_ADDED => array_keys($adapter->getReviewersAddedByHerald()), DifferentialReviewerStatus::STATUS_BLOCKING => array_keys($adapter->getBlockingReviewersAddedByHerald()), ); $old_reviewers = $object->getReviewerStatus(); $old_reviewers = mpull($old_reviewers, null, 'getReviewerPHID'); $value = array(); foreach ($reviewers as $status => $phids) { foreach ($phids as $phid) { if ($phid == $object->getAuthorPHID()) { // Don't try to add the revision's author as a reviewer, since this // isn't valid and doesn't make sense. continue; } // If the target is already a reviewer, don't try to change anything // if their current status is at least as strong as the new status. // For example, don't downgrade an "Accepted" to a "Blocking Reviewer". $old_reviewer = idx($old_reviewers, $phid); if ($old_reviewer) { $old_status = $old_reviewer->getStatus(); $old_strength = DifferentialReviewerStatus::getStatusStrength( $old_status); $new_strength = DifferentialReviewerStatus::getStatusStrength( $status); if ($new_strength <= $old_strength) { continue; } } $value['+'][$phid] = array( 'data' => array( 'status' => $status, ), ); } } if ($value) { $edge_reviewer = DifferentialRevisionHasReviewerEdgeType::EDGECONST; $xactions[] = id(new DifferentialTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) ->setMetadataValue('edge:type', $edge_reviewer) ->setNewValue($value); } // Require legalpad document signatures. $legal_phids = $adapter->getRequiredSignatureDocumentPHIDs(); if ($legal_phids) { // We only require signatures of documents which have not already // been signed. In general, this reduces the amount of churn that // signature rules cause. $signatures = id(new LegalpadDocumentSignatureQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withDocumentPHIDs($legal_phids) ->withSignerPHIDs(array($object->getAuthorPHID())) ->execute(); $signed_phids = mpull($signatures, 'getDocumentPHID'); $legal_phids = array_diff($legal_phids, $signed_phids); // If we still have something to trigger, add the edges. if ($legal_phids) { $edge_legal = LegalpadObjectNeedsSignatureEdgeType::EDGECONST; $xactions[] = id(new DifferentialTransaction()) ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) ->setMetadataValue('edge:type', $edge_legal) ->setNewValue( array( '+' => array_fuse($legal_phids), )); } } // Save extra email PHIDs for later. $email_phids = $adapter->getEmailPHIDsAddedByHerald(); $this->heraldEmailPHIDs = array_keys($email_phids); // Apply build plans. HarbormasterBuildable::applyBuildPlans( $adapter->getDiff()->getPHID(), $adapter->getPHID(), $adapter->getBuildPlans()); return $xactions; } /** * 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) { $repository = $revision->getRepository(); if (!$repository) { // The repository where the code 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)); } } $changesets = $diff->getChangesets(); $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)); } } private function renderAffectedFilesForMail(DifferentialDiff $diff) { $changesets = $diff->getChangesets(); $filenames = mpull($changesets, 'getDisplayFilename'); sort($filenames); $count = count($filenames); $max = 250; if ($count > $max) { $filenames = array_slice($filenames, 0, $max); $filenames[] = pht('(%d more files...)', ($count - $max)); } return implode("\n", $filenames); } private function renderPatchHTMLForMail($patch) { return phutil_tag('pre', array('style' => 'font-family: monospace;'), $patch); } private function renderPatchForMail(DifferentialDiff $diff) { $format = PhabricatorEnv::getEnvConfig('metamta.differential.patch-format'); $patch = id(new DifferentialRawDiffRenderer()) ->setViewer($this->getActor()) ->setFormat($format) ->setChangesets($diff->getChangesets()) ->buildPatch(); $section = new PhabricatorMetaMTAMailSection(); $section->addHTMLFragment($this->renderPatchHTMLForMail($patch)); $section->addPlaintextFragment($patch); return $section; } } diff --git a/src/applications/differential/query/DifferentialTransactionQuery.php b/src/applications/differential/query/DifferentialTransactionQuery.php index 60fea527c1..64ef271655 100644 --- a/src/applications/differential/query/DifferentialTransactionQuery.php +++ b/src/applications/differential/query/DifferentialTransactionQuery.php @@ -1,45 +1,52 @@ establishConnection('r'); $phids = queryfx_all( $conn_r, 'SELECT phid FROM %T WHERE revisionPHID = %s AND authorPHID = %s - AND transactionPHID IS NULL', + AND transactionPHID IS NULL + AND isDeleted = 0', $table->getTableName(), $revision->getPHID(), $viewer->getPHID()); $phids = ipull($phids, 'phid'); if (!$phids) { return array(); } - return id(new PhabricatorApplicationTransactionCommentQuery()) + $comments = id(new PhabricatorApplicationTransactionCommentQuery()) ->setTemplate(new DifferentialTransactionComment()) ->setViewer($viewer) ->withPHIDs($phids) ->execute(); + + $comments = PhabricatorInlineCommentController::loadAndAttachReplies( + $viewer, + $comments); + + return $comments; } } diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php index fd406f3969..0d1e6ea33a 100644 --- a/src/applications/differential/storage/DifferentialTransactionComment.php +++ b/src/applications/differential/storage/DifferentialTransactionComment.php @@ -1,90 +1,102 @@ replyToComment = $comment; + return $this; + } + + public function getReplyToComment() { + return $this->assertAttached($this->replyToComment); + } + protected function getConfiguration() { $config = parent::getConfiguration(); $config[self::CONFIG_COLUMN_SCHEMA] = array( 'revisionPHID' => 'phid?', 'changesetID' => 'id?', 'isNewFile' => 'bool', 'lineNumber' => 'uint32', 'lineLength' => 'uint32', 'fixedState' => 'text12?', 'hasReplies' => 'bool', 'replyToCommentPHID' => 'phid?', ) + $config[self::CONFIG_COLUMN_SCHEMA]; $config[self::CONFIG_KEY_SCHEMA] = array( 'key_draft' => array( 'columns' => array('authorPHID', 'transactionPHID'), ), 'key_changeset' => array( 'columns' => array('changesetID'), ), 'key_revision' => array( 'columns' => array('revisionPHID'), ), ) + $config[self::CONFIG_KEY_SCHEMA]; return $config; } public function shouldUseMarkupCache($field) { // Only cache submitted comments. return ($this->getTransactionPHID() != null); } public static function sortAndGroupInlines( array $inlines, array $changesets) { assert_instances_of($inlines, 'DifferentialTransaction'); assert_instances_of($changesets, 'DifferentialChangeset'); $changesets = mpull($changesets, null, 'getID'); $changesets = msort($changesets, 'getFilename'); // Group the changesets by file and reorder them by display order. $inline_groups = array(); foreach ($inlines as $inline) { $changeset_id = $inline->getComment()->getChangesetID(); $inline_groups[$changeset_id][] = $inline; } $inline_groups = array_select_keys($inline_groups, array_keys($changesets)); foreach ($inline_groups as $changeset_id => $group) { // Sort the group of inlines by line number. $items = array(); foreach ($group as $inline) { $comment = $inline->getComment(); $num = $comment->getLineNumber(); $len = $comment->getLineLength(); $id = $comment->getID(); $items[] = array( 'inline' => $inline, 'sort' => sprintf('~%010d%010d%010d', $num, $len, $id), ); } $items = isort($items, 'sort'); $items = ipull($items, 'inline'); $inline_groups[$changeset_id] = $items; } return $inline_groups; } } diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index a23efbc9a4..c400b965d2 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -1,312 +1,352 @@ commentID; } public function getOperation() { return $this->operation; } public function getCommentText() { return $this->commentText; } public function getLineLength() { return $this->lineLength; } public function getLineNumber() { return $this->lineNumber; } public function getIsOnRight() { return $this->isOnRight; } public function getChangesetID() { return $this->changesetID; } public function getIsNewFile() { return $this->isNewFile; } public function setRenderer($renderer) { $this->renderer = $renderer; return $this; } public function getRenderer() { return $this->renderer; } public function setReplyToCommentPHID($phid) { $this->replyToCommentPHID = $phid; return $this; } public function getReplyToCommentPHID() { return $this->replyToCommentPHID; } public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); $this->readRequestParameters(); switch ($this->getOperation()) { case 'delete': $inline = $this->loadCommentForEdit($this->getCommentID()); if ($request->isFormPost()) { $this->deleteComment($inline); return $this->buildEmptyResponse(); } $dialog = new AphrontDialogView(); $dialog->setUser($user); $dialog->setSubmitURI($request->getRequestURI()); $dialog->setTitle(pht('Really delete this comment?')); $dialog->addHiddenInput('id', $this->getCommentID()); $dialog->addHiddenInput('op', 'delete'); $dialog->appendChild( phutil_tag('p', array(), pht('Delete this inline comment?'))); $dialog->addCancelButton('#'); $dialog->addSubmitButton(pht('Delete')); return id(new AphrontDialogResponse())->setDialog($dialog); case 'edit': $inline = $this->loadCommentForEdit($this->getCommentID()); $text = $this->getCommentText(); if ($request->isFormPost()) { if (strlen($text)) { $inline->setContent($text); $this->saveComment($inline); return $this->buildRenderedCommentResponse( $inline, $this->getIsOnRight()); } else { $this->deleteComment($inline); return $this->buildEmptyResponse(); } } $edit_dialog = $this->buildEditDialog(); $edit_dialog->setTitle(pht('Edit Inline Comment')); $edit_dialog->addHiddenInput('id', $this->getCommentID()); $edit_dialog->addHiddenInput('op', 'edit'); $edit_dialog->appendChild( $this->renderTextArea( nonempty($text, $inline->getContent()))); $view = $this->buildScaffoldForView($edit_dialog); return id(new AphrontAjaxResponse()) ->setContent($view->render()); case 'create': $text = $this->getCommentText(); if (!$request->isFormPost() || !strlen($text)) { return $this->buildEmptyResponse(); } $inline = $this->createComment() ->setChangesetID($this->getChangesetID()) ->setAuthorPHID($user->getPHID()) ->setLineNumber($this->getLineNumber()) ->setLineLength($this->getLineLength()) ->setIsNewFile($this->getIsNewFile()) ->setContent($text); if ($this->getReplyToCommentPHID()) { $inline->setReplyToCommentPHID($this->getReplyToCommentPHID()); } $this->saveComment($inline); return $this->buildRenderedCommentResponse( $inline, $this->getIsOnRight()); case 'reply': default: $edit_dialog = $this->buildEditDialog(); if ($this->getOperation() == 'reply') { $inline = $this->loadComment($this->getCommentID()); $edit_dialog->setTitle(pht('Reply to Inline Comment')); $changeset = $inline->getChangesetID(); $is_new = $inline->getIsNewFile(); $number = $inline->getLineNumber(); $length = $inline->getLineLength(); } else { $edit_dialog->setTitle(pht('New Inline Comment')); $changeset = $this->getChangesetID(); $is_new = $this->getIsNewFile(); $number = $this->getLineNumber(); $length = $this->getLineLength(); } $edit_dialog->addHiddenInput('op', 'create'); $edit_dialog->addHiddenInput('is_new', $is_new); $edit_dialog->addHiddenInput('number', $number); $edit_dialog->addHiddenInput('length', $length); $text_area = $this->renderTextArea($this->getCommentText()); $edit_dialog->appendChild($text_area); $view = $this->buildScaffoldForView($edit_dialog); return id(new AphrontAjaxResponse()) ->setContent($view->render()); } } private function readRequestParameters() { $request = $this->getRequest(); // NOTE: This isn't necessarily a DifferentialChangeset ID, just an // application identifier for the changeset. In Diffusion, it's a Path ID. $this->changesetID = $request->getInt('changesetID'); $this->isNewFile = (int)$request->getBool('is_new'); $this->isOnRight = $request->getBool('on_right'); $this->lineNumber = $request->getInt('number'); $this->lineLength = $request->getInt('length'); $this->commentText = $request->getStr('text'); $this->commentID = $request->getInt('id'); $this->operation = $request->getStr('op'); $this->renderer = $request->getStr('renderer'); $this->replyToCommentPHID = $request->getStr('replyToCommentPHID'); if ($this->getReplyToCommentPHID()) { $reply_phid = $this->getReplyToCommentPHID(); $reply_comment = $this->loadCommentByPHID($reply_phid); if (!$reply_comment) { throw new Exception( pht('Failed to load comment "%s".', $reply_phid)); } if ($reply_comment->getChangesetID() != $this->getChangesetID()) { throw new Exception( pht( 'Comment "%s" belongs to wrong changeset (%s vs %s).', $reply_phid, $reply_comment->getChangesetID(), $this->getChangesetID())); } } } private function buildEditDialog() { $request = $this->getRequest(); $user = $request->getUser(); $edit_dialog = id(new PHUIDiffInlineCommentEditView()) ->setUser($user) ->setSubmitURI($request->getRequestURI()) ->setIsOnRight($this->getIsOnRight()) ->setIsNewFile($this->getIsNewFile()) ->setNumber($this->getLineNumber()) ->setLength($this->getLineLength()) ->setRenderer($this->getRenderer()) ->setReplyToCommentPHID($this->getReplyToCommentPHID()) ->setChangesetID($this->getChangesetID()); return $edit_dialog; } private function buildEmptyResponse() { return id(new AphrontAjaxResponse()) ->setContent( array( 'markup' => '', )); } private function buildRenderedCommentResponse( PhabricatorInlineCommentInterface $inline, $on_right) { $request = $this->getRequest(); $user = $request->getUser(); $engine = new PhabricatorMarkupEngine(); $engine->setViewer($user); $engine->addObject( $inline, PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); $engine->process(); $phids = array($user->getPHID()); $handles = $this->loadViewerHandles($phids); $view = id(new PHUIDiffInlineCommentDetailView()) ->setInlineComment($inline) ->setIsOnRight($on_right) ->setMarkupEngine($engine) ->setHandles($handles) ->setEditable(true); $view = $this->buildScaffoldForView($view); return id(new AphrontAjaxResponse()) ->setContent( array( 'inlineCommentID' => $inline->getID(), 'markup' => $view->render(), )); } private function renderTextArea($text) { return id(new PhabricatorRemarkupControl()) ->setUser($this->getRequest()->getUser()) ->setSigil('differential-inline-comment-edit-textarea') ->setName('text') ->setValue($text) ->setDisableFullScreen(true); } private function buildScaffoldForView(PHUIDiffInlineCommentView $view) { $renderer = DifferentialChangesetHTMLRenderer::getHTMLRendererByKey( $this->getRenderer()); $view = $renderer->getRowScaffoldForInline($view); return id(new PHUIDiffInlineCommentTableScaffold()) ->addRowScaffold($view); } + public static function loadAndAttachReplies( + PhabricatorUser $viewer, + array $comments) { + // TODO: This code doesn't really belong here, but we don't have a much + // better place to put it at the moment. + + if (!$comments) { + return $comments; + } + + $template = head($comments); + + $reply_phids = array(); + foreach ($comments as $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + if ($reply_phid) { + $reply_phids[] = $reply_phid; + } + } + + if ($reply_phids) { + $reply_comments = id(new PhabricatorApplicationTransactionCommentQuery()) + ->setTemplate($template) + ->setViewer($viewer) + ->withPHIDs($reply_phids) + ->execute(); + $reply_comments = mpull($reply_comments, null, 'getPHID'); + } else { + $reply_comments = array(); + } + + foreach ($comments as $comment) { + $reply_phid = $comment->getReplyToCommentPHID(); + $reply = idx($reply_comments, $reply_phid); + $comment->attachReplyToComment($reply); + } + + return $comments; + } + }