mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-22 23:02:42 +01:00
Reduce STATUS_CLOSED (now internally "Published") revision status callsites
Summary: Ref T2543. Add `isPublished()` to mean: exactly the status 'closed', which is now interally called 'published', but still shown as 'closed' to users. We have some callsites which are about "exactly that status", vs "any 'closed' status", e.g. including "abandoned". This also introduces `isChangePlanned()`, which felt less awkward than `isChangesPlanned()` but more consistent than `hasChangesPlanned()` or `isStatusChangesPlanned()` or similar. Test Plan: `grep`, loaded revisions, requested review. Reviewers: chad Reviewed By: chad Maniphest Tasks: T2543 Differential Revision: https://secure.phabricator.com/D18341
This commit is contained in:
parent
70088f7eec
commit
03ab7224bb
7 changed files with 21 additions and 14 deletions
|
@ -69,7 +69,7 @@ final class DifferentialUpdateRevisionConduitAPIMethod
|
||||||
throw new ConduitException('ERR_BAD_REVISION');
|
throw new ConduitException('ERR_BAD_REVISION');
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($revision->getStatus() == ArcanistDifferentialRevisionStatus::CLOSED) {
|
if ($revision->isPublished()) {
|
||||||
throw new ConduitException('ERR_CLOSED');
|
throw new ConduitException('ERR_CLOSED');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -44,6 +44,14 @@ final class DifferentialRevisionStatus extends Phobject {
|
||||||
return ($this->key === self::NEEDS_REVIEW);
|
return ($this->key === self::NEEDS_REVIEW);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function isPublished() {
|
||||||
|
return ($this->key === self::PUBLISHED);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function isChangePlanned() {
|
||||||
|
return ($this->key === self::CHANGES_PLANNED);
|
||||||
|
}
|
||||||
|
|
||||||
public static function newForLegacyStatus($legacy_status) {
|
public static function newForLegacyStatus($legacy_status) {
|
||||||
$result = new self();
|
$result = new self();
|
||||||
|
|
||||||
|
|
|
@ -628,6 +628,14 @@ final class DifferentialRevision extends DifferentialDAO
|
||||||
return $this->getStatusObject()->isNeedsReview();
|
return $this->getStatusObject()->isNeedsReview();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function isChangePlanned() {
|
||||||
|
return $this->getStatusObject()->isChangePlanned();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function isPublished() {
|
||||||
|
return $this->getStatusObject()->isPublished();
|
||||||
|
}
|
||||||
|
|
||||||
public function getStatusIcon() {
|
public function getStatusIcon() {
|
||||||
return $this->getStatusObject()->getIcon();
|
return $this->getStatusObject()->getIcon();
|
||||||
}
|
}
|
||||||
|
|
|
@ -56,9 +56,7 @@ final class DifferentialRevisionPlanChangesTransaction
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function validateAction($object, PhabricatorUser $viewer) {
|
protected function validateAction($object, PhabricatorUser $viewer) {
|
||||||
$status_planned = ArcanistDifferentialRevisionStatus::CHANGES_PLANNED;
|
if ($object->isChangePlanned()) {
|
||||||
|
|
||||||
if ($object->getStatus() == $status_planned) {
|
|
||||||
throw new Exception(
|
throw new Exception(
|
||||||
pht(
|
pht(
|
||||||
'You can not request review of this revision because this '.
|
'You can not request review of this revision because this '.
|
||||||
|
|
|
@ -39,10 +39,7 @@ final class DifferentialRevisionReopenTransaction
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function validateAction($object, PhabricatorUser $viewer) {
|
protected function validateAction($object, PhabricatorUser $viewer) {
|
||||||
// Note that we're testing for "Closed", exactly, not just any closed
|
if ($object->isPublished()) {
|
||||||
// status.
|
|
||||||
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
|
|
||||||
if ($object->getStatus() != $status_closed) {
|
|
||||||
throw new Exception(
|
throw new Exception(
|
||||||
pht(
|
pht(
|
||||||
'You can not reopen this revision because it is not closed. '.
|
'You can not reopen this revision because it is not closed. '.
|
||||||
|
|
|
@ -37,8 +37,7 @@ final class DifferentialRevisionRequestReviewTransaction
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function validateAction($object, PhabricatorUser $viewer) {
|
protected function validateAction($object, PhabricatorUser $viewer) {
|
||||||
$status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
|
if ($object->isNeedsReview()) {
|
||||||
if ($object->getStatus() == $status_review) {
|
|
||||||
throw new Exception(
|
throw new Exception(
|
||||||
pht(
|
pht(
|
||||||
'You can not request review of this revision because this '.
|
'You can not request review of this revision because this '.
|
||||||
|
|
|
@ -203,10 +203,7 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
|
||||||
$revision->getID(),
|
$revision->getID(),
|
||||||
$commit->getPHID());
|
$commit->getPHID());
|
||||||
|
|
||||||
$status_closed = ArcanistDifferentialRevisionStatus::CLOSED;
|
$should_close = !$revision->isPublished() && $should_autoclose;
|
||||||
$should_close = ($revision->getStatus() != $status_closed) &&
|
|
||||||
$should_autoclose;
|
|
||||||
|
|
||||||
if ($should_close) {
|
if ($should_close) {
|
||||||
$commit_close_xaction = id(new DifferentialTransaction())
|
$commit_close_xaction = id(new DifferentialTransaction())
|
||||||
->setTransactionType(DifferentialTransaction::TYPE_ACTION)
|
->setTransactionType(DifferentialTransaction::TYPE_ACTION)
|
||||||
|
|
Loading…
Reference in a new issue