Do not publish/notify about commits which are not reachable from any "Autoclose" ref
Summary: Depends on D20418. Ref T13277. Fixes T11314. Currently, when you push commits to some arbitrary ref or tag (like `refs/pull/123` on GitHub, `refs/tags/phabricator/diff/123` on Phabricator, or `refs/changes/whatever` on Gerrit), we do not "autoclose" related objects. This means that we don't process `Ref T123` to create links to tasks, and don't process `Differential Revision: xyz` to close revisions. However, we //do// still publish these commits. "Publish" means: trigger audits, publish feed stories, and run Herald rules. - Stop publishing these commits. - In the UI, show these commits as "Not Permanent" with a note that they are "Not [on any permanent branch]." These commits will publish and autoclose if they ever become reachable from an "autoclose" ref (most commonly, if they are later merged to "master"). Test Plan: - Pushed a commit to `refs/tags/quack`. - Before: got a feed story. - After: no feed story, UI shows commit as "Not Permanent". Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13277, T11314 Differential Revision: https://secure.phabricator.com/D20419
This commit is contained in:
		| @@ -718,7 +718,7 @@ final class PhabricatorAuditEditor | ||||
|       switch ($xaction->getTransactionType()) { | ||||
|         case PhabricatorAuditTransaction::TYPE_COMMIT: | ||||
|           $repository = $object->getRepository(); | ||||
|           if (!$repository->shouldPublish()) { | ||||
|           if (!$repository->shouldPublishCommit($object)) { | ||||
|             return false; | ||||
|           } | ||||
|           return true; | ||||
| @@ -779,7 +779,7 @@ final class PhabricatorAuditEditor | ||||
|     // TODO: They should, and then we should simplify this. | ||||
|     $repository = $object->getRepository($assert_attached = false); | ||||
|     if ($repository != PhabricatorLiskDAO::ATTACHABLE) { | ||||
|       if (!$repository->shouldPublish()) { | ||||
|       if (!$repository->shouldPublishCommit($object)) { | ||||
|         return false; | ||||
|       } | ||||
|     } | ||||
|   | ||||
| @@ -240,6 +240,19 @@ final class DiffusionCommitController extends DiffusionController { | ||||
|             'reachable from any branch, tag, or ref.'); | ||||
|         } | ||||
|       } | ||||
|       if (!$commit->isPermanentCommit()) { | ||||
|         $nonpermanent_tag = id(new PHUITagView()) | ||||
|           ->setType(PHUITagView::TYPE_SHADE) | ||||
|           ->setName(pht('Not Permanent')) | ||||
|           ->setColor(PHUITagView::COLOR_ORANGE); | ||||
|  | ||||
|         $header->addTag($nonpermanent_tag); | ||||
|  | ||||
|         $this->commitErrors[] = pht( | ||||
|           'This commit is not reachable from any permanent branch, tag, '. | ||||
|           'or ref.'); | ||||
|       } | ||||
|  | ||||
|  | ||||
|       if ($this->getCommitErrors()) { | ||||
|         $error_panel = id(new PHUIInfoView()) | ||||
|   | ||||
| @@ -1055,6 +1055,18 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO | ||||
|     return true; | ||||
|   } | ||||
|  | ||||
|   public function shouldPublishCommit(PhabricatorRepositoryCommit $commit) { | ||||
|     if (!$this->shouldPublish()) { | ||||
|       return false; | ||||
|     } | ||||
|  | ||||
|     if (!$commit->isPermanentCommit()) { | ||||
|       return false; | ||||
|     } | ||||
|  | ||||
|     return true; | ||||
|   } | ||||
|  | ||||
|  | ||||
| /* -(  Autoclose  )---------------------------------------------------------- */ | ||||
|  | ||||
| @@ -1152,8 +1164,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO | ||||
|         throw new Exception(pht('Unrecognized version control system.')); | ||||
|     } | ||||
|  | ||||
|     $closeable_flag = PhabricatorRepositoryCommit::IMPORTED_CLOSEABLE; | ||||
|     if (!$commit->isPartiallyImported($closeable_flag)) { | ||||
|     if (!$commit->isPermanentCommit()) { | ||||
|       return self::BECAUSE_NOT_ON_AUTOCLOSE_BRANCH; | ||||
|     } | ||||
|  | ||||
|   | ||||
| @@ -496,6 +496,10 @@ final class PhabricatorRepositoryCommit | ||||
|     return $this->getAuditStatusObject()->isAudited(); | ||||
|   } | ||||
|  | ||||
|   public function isPermanentCommit() { | ||||
|     return (bool)$this->isPartiallyImported(self::IMPORTED_CLOSEABLE); | ||||
|   } | ||||
|  | ||||
| /* -(  PhabricatorPolicyInterface  )----------------------------------------- */ | ||||
|  | ||||
|   public function getCapabilities() { | ||||
|   | ||||
| @@ -30,7 +30,7 @@ final class PhabricatorRepositoryCommitOwnersWorker | ||||
|     PhabricatorRepositoryCommit $commit) { | ||||
|     $viewer = PhabricatorUser::getOmnipotentUser(); | ||||
|  | ||||
|     if (!$repository->shouldPublish()) { | ||||
|     if (!$repository->shouldPublishCommit($commit)) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley