1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Partially decouple revision broadcasting from revision draft state

Summary:
Depends on D19283. Ref T13110. To enable "Changes Planned + But, Still A Draft" and "Abandoned + But, Never Promoted From Draft" states, decouple the "broadcast" flag from the "draft" state.

Broadcast behavior is now based only on the `shouldBroadcast` flag, and revisions in any state may have this flag.

Revisions gain this flag when created as a non-draft, or when they leave the draft state for the first time.

There are probably still some ways you can get the wrong result here -- maybe abandon + update -- but those can be cleaned up as they arise.

Test Plan: Kinda poked it a bit but I'll vet this more heavily at the end of this sequence.

Maniphest Tasks: T13110

Differential Revision: https://secure.phabricator.com/D19284
This commit is contained in:
epriestley 2018-04-03 08:15:05 -07:00
parent 3b5a7d1c88
commit 38e788c99a
3 changed files with 12 additions and 11 deletions

View file

@ -10,7 +10,7 @@ final class DifferentialTransactionEditor
private $hasReviewTransaction = false;
private $affectedPaths;
private $firstBroadcast = false;
private $wasDraft = false;
private $wasBroadcasting;
public function getEditorApplicationClass() {
return 'PhabricatorDifferentialApplication';
@ -137,7 +137,7 @@ final class DifferentialTransactionEditor
}
}
$this->wasDraft = $object->isDraft();
$this->wasBroadcasting = $object->getShouldBroadcast();
return parent::expandTransactions($object, $xactions);
}
@ -1594,17 +1594,13 @@ final class DifferentialTransactionEditor
// email so the mail gets enriched with "SUMMARY" and "TEST PLAN".
$is_new = $this->getIsNewObject();
$was_draft = $this->wasDraft;
$was_broadcasting = $this->wasBroadcasting;
if (!$object->isDraft() && ($was_draft || $is_new)) {
if (!$object->getShouldBroadcast()) {
if ($object->getShouldBroadcast()) {
if (!$was_broadcasting || $is_new) {
// Mark this as the first broadcast we're sending about the revision
// so mail can generate specially.
$this->firstBroadcast = true;
$object
->setShouldBroadcast(true)
->save();
}
}

View file

@ -75,8 +75,10 @@ final class DifferentialRevision extends DifferentialDAO
if (PhabricatorEnv::getEnvConfig('phabricator.show-prototypes')) {
$initial_state = DifferentialRevisionStatus::DRAFT;
$should_broadcast = false;
} else {
$initial_state = DifferentialRevisionStatus::NEEDS_REVIEW;
$should_broadcast = true;
}
return id(new DifferentialRevision())
@ -85,7 +87,8 @@ final class DifferentialRevision extends DifferentialDAO
->attachRepository(null)
->attachActiveDiff(null)
->attachReviewers(array())
->setModernRevisionStatus($initial_state);
->setModernRevisionStatus($initial_state)
->setShouldBroadcast($should_broadcast);
}
protected function getConfiguration() {

View file

@ -37,7 +37,9 @@ final class DifferentialRevisionRequestReviewTransaction
public function applyInternalEffects($object, $value) {
$status_review = DifferentialRevisionStatus::NEEDS_REVIEW;
$object->setModernRevisionStatus($status_review);
$object
->setModernRevisionStatus($status_review)
->setShouldBroadcast(true);
}
protected function validateAction($object, PhabricatorUser $viewer) {