diff --git a/scripts/repository/rebuild_summaries.php b/scripts/repository/rebuild_summaries.php deleted file mode 100755 index de3d5acca0..0000000000 --- a/scripts/repository/rebuild_summaries.php +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env php -establishConnection('w'); -$sizes = queryfx_all( - $conn_w, - 'SELECT repositoryID, count(*) N FROM %T GROUP BY repositoryID', - $commit->getTableName()); -$sizes = ipull($sizes, 'N', 'repositoryID'); - -$maxes = queryfx_all( - $conn_w, - 'SELECT repositoryID, max(epoch) maxEpoch FROM %T GROUP BY repositoryID', - $commit->getTableName()); -$maxes = ipull($maxes, 'maxEpoch', 'repositoryID'); - - -$repository_ids = array_keys($sizes + $maxes); - -echo pht('Updating %d repositories', count($repository_ids)); - -foreach ($repository_ids as $repository_id) { - $last_commit = queryfx_one( - $conn_w, - 'SELECT id FROM %T WHERE repositoryID = %d AND epoch = %d LIMIT 1', - $commit->getTableName(), - $repository_id, - idx($maxes, $repository_id, 0)); - if ($last_commit) { - $last_commit = $last_commit['id']; - } else { - $last_commit = 0; - } - queryfx( - $conn_w, - 'INSERT INTO %T (repositoryID, lastCommitID, size, epoch) - VALUES (%d, %d, %d, %d) ON DUPLICATE KEY UPDATE - lastCommitID = VALUES(lastCommitID), - size = VALUES(size), - epoch = VALUES(epoch)', - PhabricatorRepository::TABLE_SUMMARY, - $repository_id, - $last_commit, - idx($sizes, $repository_id, 0), - idx($maxes, $repository_id, 0)); - echo '.'; -} -echo "\n".pht('Done.')."\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 3abc10ef49..9774741060 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -313,6 +313,7 @@ phutil_register_library_map(array( 'ConduitCallTestCase' => 'applications/conduit/call/__tests__/ConduitCallTestCase.php', 'ConduitColumnsParameterType' => 'applications/conduit/parametertype/ConduitColumnsParameterType.php', 'ConduitConnectConduitAPIMethod' => 'applications/conduit/method/ConduitConnectConduitAPIMethod.php', + 'ConduitConstantDescription' => 'applications/conduit/data/ConduitConstantDescription.php', 'ConduitEpochParameterType' => 'applications/conduit/parametertype/ConduitEpochParameterType.php', 'ConduitException' => 'applications/conduit/protocol/exception/ConduitException.php', 'ConduitGetCapabilitiesConduitAPIMethod' => 'applications/conduit/method/ConduitGetCapabilitiesConduitAPIMethod.php', @@ -5628,6 +5629,7 @@ phutil_register_library_map(array( 'ConduitCallTestCase' => 'PhabricatorTestCase', 'ConduitColumnsParameterType' => 'ConduitParameterType', 'ConduitConnectConduitAPIMethod' => 'ConduitAPIMethod', + 'ConduitConstantDescription' => 'Phobject', 'ConduitEpochParameterType' => 'ConduitParameterType', 'ConduitException' => 'Exception', 'ConduitGetCapabilitiesConduitAPIMethod' => 'ConduitAPIMethod', diff --git a/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php b/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php index 31bb2c22f7..e559862c83 100644 --- a/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php +++ b/src/applications/audit/constants/PhabricatorAuditCommitStatusConstants.php @@ -2,6 +2,9 @@ final class PhabricatorAuditCommitStatusConstants extends Phobject { + private $key; + private $spec = array(); + const NONE = 0; const NEEDS_AUDIT = 1; const CONCERN_RAISED = 2; @@ -9,17 +12,57 @@ final class PhabricatorAuditCommitStatusConstants extends Phobject { const FULLY_AUDITED = 4; const NEEDS_VERIFICATION = 5; - public static function getStatusNameMap() { - $map = array( - self::NONE => pht('No Audits'), - self::NEEDS_AUDIT => pht('Audit Required'), - self::CONCERN_RAISED => pht('Concern Raised'), - self::NEEDS_VERIFICATION => pht('Needs Verification'), - self::PARTIALLY_AUDITED => pht('Partially Audited'), - self::FULLY_AUDITED => pht('Audited'), - ); + const MODERN_NONE = 'none'; + const MODERN_NEEDS_AUDIT = 'needs-audit'; + const MODERN_CONCERN_RAISED = 'concern-raised'; + const MODERN_PARTIALLY_AUDITED = 'partially-audited'; + const MODERN_AUDITED = 'audited'; + const MODERN_NEEDS_VERIFICATION = 'needs-verification'; - return $map; + public static function newForLegacyStatus($status) { + $map = self::getMap(); + + foreach ($map as $key => $spec) { + if (idx($spec, 'legacy') == $status) { + return self::newForStatus($key); + } + } + + return self::newForStatus($status); + } + + public static function newForStatus($status) { + $result = new self(); + + $result->key = $status; + + $map = self::getMap(); + if (isset($map[$status])) { + $result->spec = $map[$status]; + } + + return $result; + } + + public function getKey() { + return $this->key; + } + + public function getIcon() { + return idx($this->spec, 'icon'); + } + + public function getColor() { + return idx($this->spec, 'color'); + } + + public function getName() { + return idx($this->spec, 'name', pht('Unknown ("%s")', $this->key)); + } + + public static function getStatusNameMap() { + $map = self::getMap(); + return ipull($map, 'name', 'legacy'); } public static function getStatusName($code) { @@ -27,66 +70,71 @@ final class PhabricatorAuditCommitStatusConstants extends Phobject { } public static function getOpenStatusConstants() { - return array( - self::CONCERN_RAISED, - self::NEEDS_AUDIT, - self::NEEDS_VERIFICATION, - self::PARTIALLY_AUDITED, - ); + $constants = array(); + foreach (self::getMap() as $map) { + if (!$map['closed']) { + $constants[] = $map['legacy']; + } + } + return $constants; } public static function getStatusColor($code) { - switch ($code) { - case self::CONCERN_RAISED: - $color = 'red'; - break; - case self::NEEDS_AUDIT: - $color = 'orange'; - break; - case self::PARTIALLY_AUDITED: - $color = 'yellow'; - break; - case self::FULLY_AUDITED: - $color = 'green'; - break; - case self::NONE: - $color = 'bluegrey'; - break; - case self::NEEDS_VERIFICATION: - $color = 'indigo'; - break; - default: - $color = null; - break; - } - return $color; + $map = self::getMap(); + $map = ipull($map, 'color', 'legacy'); + return idx($map, $code); } public static function getStatusIcon($code) { - switch ($code) { - case self::CONCERN_RAISED: - $icon = 'fa-times-circle'; - break; - case self::NEEDS_AUDIT: - $icon = 'fa-exclamation-circle'; - break; - case self::PARTIALLY_AUDITED: - $icon = 'fa-check-circle-o'; - break; - case self::FULLY_AUDITED: - $icon = 'fa-check-circle'; - break; - case self::NONE: - $icon = 'fa-check'; - break; - case self::NEEDS_VERIFICATION: - $icon = 'fa-refresh'; - break; - default: - $icon = null; - break; - } - return $icon; + $map = self::getMap(); + $map = ipull($map, 'icon', 'legacy'); + return idx($map, $code); } + private static function getMap() { + return array( + self::MODERN_NONE => array( + 'name' => pht('No Audits'), + 'legacy' => self::NONE, + 'icon' => 'fa-check', + 'color' => 'bluegrey', + 'closed' => true, + ), + self::MODERN_NEEDS_AUDIT => array( + 'name' => pht('Audit Required'), + 'legacy' => self::NEEDS_AUDIT, + 'icon' => 'fa-exclamation-circle', + 'color' => 'orange', + 'closed' => false, + ), + self::MODERN_CONCERN_RAISED => array( + 'name' => pht('Concern Raised'), + 'legacy' => self::CONCERN_RAISED, + 'icon' => 'fa-times-circle', + 'color' => 'red', + 'closed' => false, + ), + self::MODERN_PARTIALLY_AUDITED => array( + 'name' => pht('Partially Audited'), + 'legacy' => self::PARTIALLY_AUDITED, + 'icon' => 'fa-check-circle-o', + 'color' => 'yellow', + 'closed' => false, + ), + self::MODERN_AUDITED => array( + 'name' => pht('Audited'), + 'legacy' => self::FULLY_AUDITED, + 'icon' => 'fa-check-circle', + 'color' => 'green', + 'closed' => true, + ), + self::MODERN_NEEDS_VERIFICATION => array( + 'name' => pht('Needs Verification'), + 'legacy' => self::NEEDS_VERIFICATION, + 'icon' => 'fa-refresh', + 'color' => 'indigo', + 'closed' => false, + ), + ); + } } diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 984e2c1472..c04bb8185b 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -258,19 +258,31 @@ final class PhabricatorAuditEditor $this->didExpandInlineState = true; $actor_phid = $this->getActingAsPHID(); - $actor_is_author = ($object->getAuthorPHID() == $actor_phid); - if (!$actor_is_author) { - break; - } + $author_phid = $object->getAuthorPHID(); + $actor_is_author = ($actor_phid == $author_phid); $state_map = PhabricatorTransactions::getInlineStateMap(); - $inlines = id(new DiffusionDiffInlineCommentQuery()) + $query = id(new DiffusionDiffInlineCommentQuery()) ->setViewer($this->getActor()) ->withCommitPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)) + ->withFixedStates(array_keys($state_map)); + + $inlines = array(); + + $inlines[] = id(clone $query) + ->withAuthorPHIDs(array($actor_phid)) + ->withHasTransaction(false) ->execute(); + if ($actor_is_author) { + $inlines[] = id(clone $query) + ->withHasTransaciton(true) + ->execute(); + } + + $inlines = array_mergev($inlines); + if (!$inlines) { break; } diff --git a/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php b/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php index db6ce096c4..96d06e65c2 100644 --- a/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php +++ b/src/applications/audit/management/PhabricatorAuditSynchronizeManagementWorkflow.php @@ -30,11 +30,11 @@ final class PhabricatorAuditSynchronizeManagementWorkflow continue; } - $old_status = $commit->getAuditStatus(); + $old_status = $commit->getAuditStatusObject(); $commit->updateAuditStatus($commit->getAudits()); - $new_status = $commit->getAuditStatus(); + $new_status = $commit->getAuditStatusObject(); - if ($old_status == $new_status) { + if ($old_status->getKey() == $new_status->getKey()) { echo tsprintf( "%s\n", pht( @@ -46,10 +46,8 @@ final class PhabricatorAuditSynchronizeManagementWorkflow pht( 'Updating "%s": "%s" -> "%s".', $commit->getDisplayName(), - PhabricatorAuditCommitStatusConstants::getStatusName( - $old_status), - PhabricatorAuditCommitStatusConstants::getStatusName( - $new_status))); + $old_status->getName(), + $new_status->getName())); $commit->save(); } diff --git a/src/applications/audit/query/PhabricatorCommitSearchEngine.php b/src/applications/audit/query/PhabricatorCommitSearchEngine.php index 993626e1e3..e1b294a9d6 100644 --- a/src/applications/audit/query/PhabricatorCommitSearchEngine.php +++ b/src/applications/audit/query/PhabricatorCommitSearchEngine.php @@ -67,35 +67,48 @@ final class PhabricatorCommitSearchEngine ->setKey('responsiblePHIDs') ->setConduitKey('responsible') ->setAliases(array('responsible', 'responsibles', 'responsiblePHID')) - ->setDatasource(new DifferentialResponsibleDatasource()), + ->setDatasource(new DifferentialResponsibleDatasource()) + ->setDescription( + pht( + 'Find commits where given users, projects, or packages are '. + 'responsible for the next steps in the audit workflow.')), id(new PhabricatorUsersSearchField()) ->setLabel(pht('Authors')) ->setKey('authorPHIDs') ->setConduitKey('authors') - ->setAliases(array('author', 'authors', 'authorPHID')), + ->setAliases(array('author', 'authors', 'authorPHID')) + ->setDescription(pht('Find commits authored by particular users.')), id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Auditors')) ->setKey('auditorPHIDs') ->setConduitKey('auditors') ->setAliases(array('auditor', 'auditors', 'auditorPHID')) - ->setDatasource(new DiffusionAuditorFunctionDatasource()), + ->setDatasource(new DiffusionAuditorFunctionDatasource()) + ->setDescription( + pht( + 'Find commits where given users, projects, or packages are '. + 'auditors.')), id(new PhabricatorSearchCheckboxesField()) ->setLabel(pht('Audit Status')) ->setKey('statuses') ->setAliases(array('status')) - ->setOptions(PhabricatorAuditCommitStatusConstants::getStatusNameMap()), + ->setOptions(PhabricatorAuditCommitStatusConstants::getStatusNameMap()) + ->setDescription(pht('Find commits with given audit statuses.')), id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Repositories')) ->setKey('repositoryPHIDs') ->setConduitKey('repositories') ->setAliases(array('repository', 'repositories', 'repositoryPHID')) - ->setDatasource(new DiffusionRepositoryFunctionDatasource()), + ->setDatasource(new DiffusionRepositoryFunctionDatasource()) + ->setDescription(pht('Find commits in particular repositories.')), id(new PhabricatorSearchDatasourceField()) ->setLabel(pht('Packages')) ->setKey('packagePHIDs') ->setConduitKey('packages') ->setAliases(array('package', 'packages', 'packagePHID')) - ->setDatasource(new PhabricatorOwnersPackageDatasource()), + ->setDatasource(new PhabricatorOwnersPackageDatasource()) + ->setDescription( + pht('Find commits which affect given packages.')), id(new PhabricatorSearchThreeStateField()) ->setLabel(pht('Unreachable')) ->setKey('unreachable') diff --git a/src/applications/audit/view/PhabricatorAuditListView.php b/src/applications/audit/view/PhabricatorAuditListView.php index cb9fecac3a..fb56e7cd55 100644 --- a/src/applications/audit/view/PhabricatorAuditListView.php +++ b/src/applications/audit/view/PhabricatorAuditListView.php @@ -120,14 +120,11 @@ final class PhabricatorAuditListView extends AphrontView { $commit_desc = $this->getCommitDescription($commit_phid); $committed = phabricator_datetime($commit->getEpoch(), $viewer); - $status = $commit->getAuditStatus(); + $status = $commit->getAuditStatusObject(); - $status_text = - PhabricatorAuditCommitStatusConstants::getStatusName($status); - $status_color = - PhabricatorAuditCommitStatusConstants::getStatusColor($status); - $status_icon = - PhabricatorAuditCommitStatusConstants::getStatusIcon($status); + $status_text = $status->getName(); + $status_color = $status->getColor(); + $status_icon = $status->getIcon(); $author_phid = $commit->getAuthorPHID(); if ($author_phid) { diff --git a/src/applications/conduit/data/ConduitConstantDescription.php b/src/applications/conduit/data/ConduitConstantDescription.php new file mode 100644 index 0000000000..fe2af2b716 --- /dev/null +++ b/src/applications/conduit/data/ConduitConstantDescription.php @@ -0,0 +1,26 @@ +key = $key; + return $this; + } + + public function getKey() { + return $this->key; + } + + public function setValue($value) { + $this->value = $value; + return $this; + } + + public function getValue() { + return $this->value; + } + +} diff --git a/src/applications/differential/controller/DifferentialInlineCommentEditController.php b/src/applications/differential/controller/DifferentialInlineCommentEditController.php index 926158cb41..9741cc93ee 100644 --- a/src/applications/differential/controller/DifferentialInlineCommentEditController.php +++ b/src/applications/differential/controller/DifferentialInlineCommentEditController.php @@ -77,19 +77,17 @@ final class DifferentialInlineCommentEditController } protected function loadCommentForEdit($id) { - $request = $this->getRequest(); - $user = $request->getUser(); + $viewer = $this->getViewer(); $inline = $this->loadComment($id); - if (!$this->canEditInlineComment($user, $inline)) { + if (!$this->canEditInlineComment($viewer, $inline)) { throw new Exception(pht('That comment is not editable!')); } return $inline; } protected function loadCommentForDone($id) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $inline = $this->loadComment($id); if (!$inline) { @@ -120,19 +118,32 @@ final class DifferentialInlineCommentEditController throw new Exception(pht('Unable to load revision.')); } - if ($revision->getAuthorPHID() !== $viewer->getPHID()) { - throw new Exception(pht('You are not the revision owner.')); + $viewer_phid = $viewer->getPHID(); + $is_owner = ($viewer_phid == $revision->getAuthorPHID()); + $is_author = ($viewer_phid == $inline->getAuthorPHID()); + $is_draft = ($inline->isDraft()); + + if ($is_owner) { + // You own the revision, so you can mark the comment as "Done". + } else if ($is_author && $is_draft) { + // You made this comment and it's still a draft, so you can mark + // it as "Done". + } else { + throw new Exception( + pht( + 'You are not the revision owner, and this is not a draft comment '. + 'you authored.')); } return $inline; } private function canEditInlineComment( - PhabricatorUser $user, + PhabricatorUser $viewer, DifferentialInlineComment $inline) { // Only the author may edit a comment. - if ($inline->getAuthorPHID() != $user->getPHID()) { + if ($inline->getAuthorPHID() != $viewer->getPHID()) { return false; } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 9c24a50b4e..c452cb9346 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -248,19 +248,34 @@ final class DifferentialTransactionEditor $this->didExpandInlineState = true; $actor_phid = $this->getActingAsPHID(); - $actor_is_author = ($object->getAuthorPHID() == $actor_phid); - if (!$actor_is_author) { - break; - } + $author_phid = $object->getAuthorPHID(); + $actor_is_author = ($actor_phid == $author_phid); $state_map = PhabricatorTransactions::getInlineStateMap(); - $inlines = id(new DifferentialDiffInlineCommentQuery()) + $query = id(new DifferentialDiffInlineCommentQuery()) ->setViewer($this->getActor()) ->withRevisionPHIDs(array($object->getPHID())) - ->withFixedStates(array_keys($state_map)) + ->withFixedStates(array_keys($state_map)); + + $inlines = array(); + + // We're going to undraft any "done" marks on your own inlines. + $inlines[] = id(clone $query) + ->withAuthorPHIDs(array($actor_phid)) + ->withHasTransaction(false) ->execute(); + // If you're the author, we also undraft any "done" marks on other + // inlines. + if ($actor_is_author) { + $inlines[] = id(clone $query) + ->withHasTransaction(true) + ->execute(); + } + + $inlines = array_mergev($inlines); + if (!$inlines) { break; } @@ -892,6 +907,17 @@ final class DifferentialTransactionEditor array $inlines, PhabricatorMetaMTAMailBody $body) { + $limit = 100; + $limit_note = null; + if (count($inlines) > $limit) { + $limit_note = pht( + '(Showing first %s of %s inline comments.)', + new PhutilNumber($limit), + phutil_count($inlines)); + + $inlines = array_slice($inlines, 0, $limit, true); + } + $section = id(new DifferentialInlineCommentMailView()) ->setViewer($this->getActor()) ->setInlines($inlines) @@ -900,6 +926,9 @@ final class DifferentialTransactionEditor $header = pht('INLINE COMMENTS'); $section_text = "\n".$section->getPlaintext(); + if ($limit_note) { + $section_text = $limit_note."\n".$section_text; + } $style = array( 'margin: 6px 0 12px 0;', @@ -912,6 +941,16 @@ final class DifferentialTransactionEditor ), $section->getHTML()); + if ($limit_note) { + $section_html = array( + phutil_tag( + 'em', + array(), + $limit_note), + $section_html, + ); + } + $body->addPlaintextSection($header, $section_text, false); $body->addHTMLSection($header, $section_html); } diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index a432c02a96..9b169f3c67 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -1152,6 +1152,20 @@ final class DifferentialRevision extends DifferentialDAO ->setKey('testPlan') ->setType('string') ->setDescription(pht('Revision test plan.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('isDraft') + ->setType('bool') + ->setDescription( + pht( + 'True if this revision is in any draft state, and thus not '. + 'notifying reviewers and subscribers about changes.')), + id(new PhabricatorConduitSearchFieldSpecification()) + ->setKey('holdAsDraft') + ->setType('bool') + ->setDescription( + pht( + 'True if this revision is being held as a draft. It will not be '. + 'automatically submitted for review even if tests pass.')), ); } @@ -1172,6 +1186,8 @@ final class DifferentialRevision extends DifferentialDAO 'diffPHID' => $this->getActiveDiffPHID(), 'summary' => $this->getSummary(), 'testPlan' => $this->getTestPlan(), + 'isDraft' => !$this->getShouldBroadcast(), + 'holdAsDraft' => (bool)$this->getHoldAsDraft(), ); } diff --git a/src/applications/diffusion/controller/DiffusionInlineCommentController.php b/src/applications/diffusion/controller/DiffusionInlineCommentController.php index 8af9a1cd65..ce69511dde 100644 --- a/src/applications/diffusion/controller/DiffusionInlineCommentController.php +++ b/src/applications/diffusion/controller/DiffusionInlineCommentController.php @@ -50,19 +50,17 @@ final class DiffusionInlineCommentController } protected function loadCommentForEdit($id) { - $request = $this->getRequest(); - $user = $request->getUser(); + $viewer = $this->getViewer(); $inline = $this->loadComment($id); - if (!$this->canEditInlineComment($user, $inline)) { + if (!$this->canEditInlineComment($viewer, $inline)) { throw new Exception(pht('That comment is not editable!')); } return $inline; } protected function loadCommentForDone($id) { - $request = $this->getRequest(); - $viewer = $request->getUser(); + $viewer = $this->getViewer(); $inline = $this->loadComment($id); if (!$inline) { @@ -77,20 +75,32 @@ final class DiffusionInlineCommentController throw new Exception(pht('Failed to load commit.')); } - if ((!$commit->getAuthorPHID()) || - ($commit->getAuthorPHID() != $viewer->getPHID())) { - throw new Exception(pht('You can not mark this comment as complete.')); + $owner_phid = $commit->getAuthorPHID(); + $viewer_phid = $viewer->getPHID(); + $viewer_is_owner = ($owner_phid && ($owner_phid == $viewer_phid)); + $viewer_is_author = ($viewer_phid == $inline->getAuthorPHID()); + $is_draft = $inline->isDraft(); + + if ($viewer_is_owner) { + // You can mark inlines on your own commits as "Done". + } else if ($viewer_is_author && $is_draft) { + // You can mark your own unsubmitted inlines as "Done". + } else { + throw new Exception( + pht( + 'You can not mark this comment as complete: you did not author '. + 'the commit and the comment is not a draft you wrote.')); } return $inline; } private function canEditInlineComment( - PhabricatorUser $user, + PhabricatorUser $viewer, PhabricatorAuditInlineComment $inline) { // Only the author may edit a comment. - if ($inline->getAuthorPHID() != $user->getPHID()) { + if ($inline->getAuthorPHID() != $viewer->getPHID()) { return false; } diff --git a/src/applications/diffusion/view/DiffusionHistoryTableView.php b/src/applications/diffusion/view/DiffusionHistoryTableView.php index 3885bbf47c..cfd7019679 100644 --- a/src/applications/diffusion/view/DiffusionHistoryTableView.php +++ b/src/applications/diffusion/view/DiffusionHistoryTableView.php @@ -114,10 +114,10 @@ final class DiffusionHistoryTableView extends DiffusionHistoryView { 'type' => $history->getFileType(), )); - $status = $commit->getAuditStatus(); - $icon = PhabricatorAuditCommitStatusConstants::getStatusIcon($status); - $color = PhabricatorAuditCommitStatusConstants::getStatusColor($status); - $name = PhabricatorAuditCommitStatusConstants::getStatusName($status); + $status = $commit->getAuditStatusObject(); + $icon = $status->getIcon(); + $color = $status->getColor(); + $name = $status->getName(); $audit_view = id(new PHUIIconView()) ->setIcon($icon, $color) diff --git a/src/applications/drydock/operation/DrydockRepositoryOperationType.php b/src/applications/drydock/operation/DrydockRepositoryOperationType.php index 7ec3689aff..32b562a303 100644 --- a/src/applications/drydock/operation/DrydockRepositoryOperationType.php +++ b/src/applications/drydock/operation/DrydockRepositoryOperationType.php @@ -3,6 +3,8 @@ abstract class DrydockRepositoryOperationType extends Phobject { private $viewer; + private $operation; + private $interface; abstract public function applyOperation( DrydockRepositoryOperation $operation, @@ -29,6 +31,27 @@ abstract class DrydockRepositoryOperationType extends Phobject { return $this->viewer; } + final public function setOperation(DrydockRepositoryOperation $operation) { + $this->operation = $operation; + return $this; + } + + final public function getOperation() { + return $this->operation; + } + + final public function setInterface(DrydockInterface $interface) { + $this->interface = $interface; + return $this; + } + + final public function getInterface() { + if (!$this->interface) { + throw new PhutilInvalidStateException('setInterface'); + } + return $this->interface; + } + final public function getOperationConstant() { return $this->getPhobjectClassConstant('OPCONST', 32); } diff --git a/src/applications/drydock/query/DrydockRepositoryOperationQuery.php b/src/applications/drydock/query/DrydockRepositoryOperationQuery.php index fb1f6583a8..a5fbe0acc9 100644 --- a/src/applications/drydock/query/DrydockRepositoryOperationQuery.php +++ b/src/applications/drydock/query/DrydockRepositoryOperationQuery.php @@ -62,6 +62,8 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { protected function willFilterPage(array $operations) { $implementations = DrydockRepositoryOperationType::getAllOperationTypes(); + $viewer = $this->getViewer(); + foreach ($operations as $key => $operation) { $impl = idx($implementations, $operation->getOperationType()); if (!$impl) { @@ -69,7 +71,10 @@ final class DrydockRepositoryOperationQuery extends DrydockQuery { unset($operations[$key]); continue; } - $impl = clone $impl; + $impl = id(clone $impl) + ->setViewer($viewer) + ->setOperation($operation); + $operation->attachImplementation($impl); } diff --git a/src/applications/drydock/storage/DrydockRepositoryOperation.php b/src/applications/drydock/storage/DrydockRepositoryOperation.php index 4e25280c3f..809a10a116 100644 --- a/src/applications/drydock/storage/DrydockRepositoryOperation.php +++ b/src/applications/drydock/storage/DrydockRepositoryOperation.php @@ -137,9 +137,9 @@ final class DrydockRepositoryOperation extends DrydockDAO } public function applyOperation(DrydockInterface $interface) { - return $this->getImplementation()->applyOperation( - $this, - $interface); + $impl = $this->getImplementation(); + $impl->setInterface($interface); + return $impl->applyOperation($this, $interface); } public function getOperationDescription(PhabricatorUser $viewer) { diff --git a/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php b/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php index dfd603fd12..2ee9324fb6 100644 --- a/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php @@ -25,8 +25,6 @@ final class DrydockRepositoryOperationUpdateWorker private function handleUpdate(DrydockRepositoryOperation $operation) { - $viewer = $this->getViewer(); - $operation_state = $operation->getOperationState(); switch ($operation_state) { @@ -53,9 +51,6 @@ final class DrydockRepositoryOperationUpdateWorker // waiting for a lease we're holding. try { - $operation->getImplementation() - ->setViewer($viewer); - $lease = $this->loadWorkingCopyLease($operation); $interface = $lease->getInterface( diff --git a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php index ccf97451b7..e0dfe05cc0 100644 --- a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php @@ -193,7 +193,7 @@ abstract class HarbormasterBuildStepImplementation extends Phobject { * @return string String with variables replaced safely into it. */ protected function mergeVariables($function, $pattern, array $variables) { - $regexp = '@\\$\\{(?P[a-z\\./-]+)\\}@'; + $regexp = '@\\$\\{(?P[a-z\\./_-]+)\\}@'; $matches = null; preg_match_all($regexp, $pattern, $matches); diff --git a/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php b/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php index b5abd40032..c37bdc04f9 100644 --- a/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php +++ b/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php @@ -82,10 +82,10 @@ final class PhabricatorRepositoryCommitPHIDType extends PhabricatorPHIDType { $handle->setURI($commit->getURI()); $handle->setTimestamp($commit->getEpoch()); - $status = $commit->getAuditStatus(); - $icon = PhabricatorAuditCommitStatusConstants::getStatusIcon($status); - $color = PhabricatorAuditCommitStatusConstants::getStatusColor($status); - $name = PhabricatorAuditCommitStatusConstants::getStatusName($status); + $status = $commit->getAuditStatusObject(); + $icon = $status->getIcon(); + $color = $status->getColor(); + $name = $status->getName(); $handle ->setStateIcon($icon) diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php index f36869686f..447db08e6c 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -530,6 +530,11 @@ final class PhabricatorRepositoryCommit return $data->getCommitDetail('authorPHID'); } + public function getAuditStatusObject() { + $status = $this->getAuditStatus(); + return PhabricatorAuditCommitStatusConstants::newForLegacyStatus($status); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() { diff --git a/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php b/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php index 3d13f2c3d9..335ba58852 100644 --- a/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php +++ b/src/applications/search/engine/PhabricatorSearchEngineAPIMethod.php @@ -161,7 +161,9 @@ If you specify both a `queryKey` and `constraints`, the builtin or saved query will be applied first as a starting point, then any additional values in `constraints` will be applied, overwriting the defaults from the original query. -Specify constraints like this: +Different endpoints support different constraints. The constraints this method +supports are detailed below. As an example, you might specify constraints like +this: ```lang=json, name="Example Custom Constraints" { @@ -188,15 +190,26 @@ EOTEXT $fields, array('ids', 'phids')) + $fields; + $constant_lists = array(); + $rows = array(); foreach ($fields as $field) { $key = $field->getConduitKey(); $label = $field->getLabel(); + $constants = $field->newConduitConstants(); + $type_object = $field->getConduitParameterType(); if ($type_object) { $type = $type_object->getTypeName(); $description = $field->getDescription(); + if ($constants) { + $description = array( + $description, + ' ', + phutil_tag('em', array(), pht('(See table below.)')), + ); + } } else { $type = null; $description = phutil_tag('em', array(), pht('Not supported.')); @@ -208,6 +221,35 @@ EOTEXT $type, $description, ); + + if ($constants) { + $constant_lists[] = $this->buildRemarkup( + pht( + 'Constants supported by the `%s` constraint:', + 'statuses')); + + $constants_rows = array(); + foreach ($constants as $constant) { + $constants_rows[] = array( + $constant->getKey(), + $constant->getValue(), + ); + } + + $constants_table = id(new AphrontTableView($constants_rows)) + ->setHeaders( + array( + pht('Key'), + pht('Value'), + )) + ->setColumnClasses( + array( + 'pre', + 'wide', + )); + + $constant_lists[] = $constants_table; + } } $table = id(new AphrontTableView($rows)) @@ -231,7 +273,8 @@ EOTEXT ->setCollapsed(true) ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->appendChild($this->buildRemarkup($info)) - ->appendChild($table); + ->appendChild($table) + ->appendChild($constant_lists); } private function buildOrderBox( diff --git a/src/applications/search/field/PhabricatorSearchCheckboxesField.php b/src/applications/search/field/PhabricatorSearchCheckboxesField.php index e1a72f4576..0985c976ce 100644 --- a/src/applications/search/field/PhabricatorSearchCheckboxesField.php +++ b/src/applications/search/field/PhabricatorSearchCheckboxesField.php @@ -49,4 +49,16 @@ final class PhabricatorSearchCheckboxesField return new ConduitStringListParameterType(); } + public function newConduitConstants() { + $list = array(); + + foreach ($this->getOptions() as $key => $option) { + $list[] = id(new ConduitConstantDescription()) + ->setKey($key) + ->setValue($option); + } + + return $list; + } + } diff --git a/src/applications/search/field/PhabricatorSearchField.php b/src/applications/search/field/PhabricatorSearchField.php index 1a038ec665..36db0523b7 100644 --- a/src/applications/search/field/PhabricatorSearchField.php +++ b/src/applications/search/field/PhabricatorSearchField.php @@ -382,6 +382,10 @@ abstract class PhabricatorSearchField extends Phobject { return $this->enableForConduit; } + public function newConduitConstants() { + return array(); + } + /* -( Utility Methods )----------------------------------------------------- */ diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php index d7f1a9dd3a..e6572672ce 100644 --- a/src/infrastructure/diff/PhabricatorInlineCommentController.php +++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php @@ -88,7 +88,7 @@ abstract class PhabricatorInlineCommentController public function processRequest() { $request = $this->getRequest(); - $user = $request->getUser(); + $viewer = $this->getViewer(); $this->readRequestParameters(); @@ -221,7 +221,7 @@ abstract class PhabricatorInlineCommentController $inline = $this->createComment() ->setChangesetID($this->getChangesetID()) - ->setAuthorPHID($user->getPHID()) + ->setAuthorPHID($viewer->getPHID()) ->setLineNumber($this->getLineNumber()) ->setLineLength($this->getLineLength()) ->setIsNewFile($this->getIsNewFile()) @@ -231,6 +231,15 @@ abstract class PhabricatorInlineCommentController $inline->setReplyToCommentPHID($this->getReplyToCommentPHID()); } + // If you own this object, mark your own inlines as "Done" by default. + $owner_phid = $this->loadObjectOwnerPHID($inline); + if ($owner_phid) { + if ($viewer->getPHID() == $owner_phid) { + $fixed_state = PhabricatorInlineCommentInterface::STATE_DRAFT; + $inline->setFixedState($fixed_state); + } + } + $this->saveComment($inline); return $this->buildRenderedCommentResponse( @@ -313,10 +322,10 @@ abstract class PhabricatorInlineCommentController private function buildEditDialog() { $request = $this->getRequest(); - $user = $request->getUser(); + $viewer = $this->getViewer(); $edit_dialog = id(new PHUIDiffInlineCommentEditView()) - ->setUser($user) + ->setUser($viewer) ->setSubmitURI($request->getRequestURI()) ->setIsOnRight($this->getIsOnRight()) ->setIsNewFile($this->getIsNewFile()) @@ -342,22 +351,22 @@ abstract class PhabricatorInlineCommentController $on_right) { $request = $this->getRequest(); - $user = $request->getUser(); + $viewer = $this->getViewer(); $engine = new PhabricatorMarkupEngine(); - $engine->setViewer($user); + $engine->setViewer($viewer); $engine->addObject( $inline, PhabricatorInlineCommentInterface::MARKUP_FIELD_BODY); $engine->process(); - $phids = array($user->getPHID()); + $phids = array($viewer->getPHID()); $handles = $this->loadViewerHandles($phids); $object_owner_phid = $this->loadObjectOwnerPHID($inline); $view = id(new PHUIDiffInlineCommentDetailView()) - ->setUser($user) + ->setUser($viewer) ->setInlineComment($inline) ->setIsOnRight($on_right) ->setMarkupEngine($engine) @@ -378,7 +387,7 @@ abstract class PhabricatorInlineCommentController private function renderTextArea($text) { return id(new PhabricatorRemarkupControl()) - ->setUser($this->getRequest()->getUser()) + ->setViewer($this->getViewer()) ->setSigil('differential-inline-comment-edit-textarea') ->setName('text') ->setValue($text) diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php index a32197059a..4da3bbd8fa 100644 --- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php +++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php @@ -287,15 +287,24 @@ final class PHUIDiffInlineCommentDetailView $done_button = null; + $mark_done = $this->getCanMarkDone(); + + // Allow users to mark their own draft inlines as "Done". + if ($viewer_phid == $inline->getAuthorPHID()) { + if ($inline->isDraft()) { + $mark_done = true; + } + } + if (!$is_synthetic) { $draft_state = false; switch ($inline->getFixedState()) { case PhabricatorInlineCommentInterface::STATE_DRAFT: - $is_done = ($this->getCanMarkDone()); + $is_done = $mark_done; $draft_state = true; break; case PhabricatorInlineCommentInterface::STATE_UNDRAFT: - $is_done = !($this->getCanMarkDone()); + $is_done = !$mark_done; $draft_state = true; break; case PhabricatorInlineCommentInterface::STATE_DONE: @@ -309,7 +318,7 @@ final class PHUIDiffInlineCommentDetailView // If you don't have permission to mark the comment as "Done", you also // can not see the draft state. - if (!$this->getCanMarkDone()) { + if (!$mark_done) { $draft_state = false; } @@ -321,21 +330,19 @@ final class PHUIDiffInlineCommentDetailView $classes[] = 'inline-state-is-draft'; } - if ($this->getCanMarkDone()) { + if ($mark_done && !$this->preview) { $done_input = javelin_tag( 'input', array( 'type' => 'checkbox', 'checked' => ($is_done ? 'checked' : null), - 'disabled' => ($this->getCanMarkDone() ? null : 'disabled'), 'class' => 'differential-inline-done', 'sigil' => 'differential-inline-done', )); $done_button = phutil_tag( 'label', array( - 'class' => 'differential-inline-done-label '. - ($this->getCanMarkDone() ? null : 'done-is-disabled'), + 'class' => 'differential-inline-done-label ', ), array( $done_input,