diff --git a/resources/sql/autopatches/20150730.herald.6.sql b/resources/sql/autopatches/20150730.herald.6.sql new file mode 100644 index 0000000000..08188c6d4f --- /dev/null +++ b/resources/sql/autopatches/20150730.herald.6.sql @@ -0,0 +1,6 @@ +UPDATE {$NAMESPACE}_herald.herald_action a + JOIN {$NAMESPACE}_herald.herald_rule r + ON a.ruleID = r.id + SET a.action = 'legalpad.require' + WHERE r.ruleType != 'personal' + AND a.action = 'signature'; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 090dbc06e4..50df87873b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1124,6 +1124,7 @@ phutil_register_library_map(array( 'LegalpadMailReceiver' => 'applications/legalpad/mail/LegalpadMailReceiver.php', 'LegalpadObjectNeedsSignatureEdgeType' => 'applications/legalpad/edge/LegalpadObjectNeedsSignatureEdgeType.php', 'LegalpadReplyHandler' => 'applications/legalpad/mail/LegalpadReplyHandler.php', + 'LegalpadRequireSignatureHeraldAction' => 'applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php', 'LegalpadSchemaSpec' => 'applications/legalpad/storage/LegalpadSchemaSpec.php', 'LegalpadSignatureNeededByObjectEdgeType' => 'applications/legalpad/edge/LegalpadSignatureNeededByObjectEdgeType.php', 'LegalpadTransaction' => 'applications/legalpad/storage/LegalpadTransaction.php', @@ -4852,6 +4853,7 @@ phutil_register_library_map(array( 'LegalpadMailReceiver' => 'PhabricatorObjectMailReceiver', 'LegalpadObjectNeedsSignatureEdgeType' => 'PhabricatorEdgeType', 'LegalpadReplyHandler' => 'PhabricatorApplicationTransactionReplyHandler', + 'LegalpadRequireSignatureHeraldAction' => 'HeraldAction', 'LegalpadSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'LegalpadSignatureNeededByObjectEdgeType' => 'PhabricatorEdgeType', 'LegalpadTransaction' => 'PhabricatorApplicationTransaction', diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 0e8548c0a6..ff4894ec3e 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1596,43 +1596,13 @@ final class DifferentialTransactionEditor HeraldAdapter $adapter, HeraldTranscript $transcript) { - $xactions = array(); - - // 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), - )); - } - } - // Apply build plans. HarbormasterBuildable::applyBuildPlans( $adapter->getDiff()->getPHID(), $adapter->getPHID(), $adapter->getBuildPlans()); - return $xactions; + return array(); } /** diff --git a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php index 9166e0e060..ce84a209ca 100644 --- a/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php +++ b/src/applications/differential/herald/HeraldDifferentialRevisionAdapter.php @@ -6,7 +6,6 @@ final class HeraldDifferentialRevisionAdapter protected $revision; protected $buildPlans = array(); - protected $requiredSignatureDocumentPHIDs = array(); protected $affectedPackages; protected $changesets; @@ -80,10 +79,6 @@ final class HeraldDifferentialRevisionAdapter return $object; } - public function getRequiredSignatureDocumentPHIDs() { - return $this->requiredSignatureDocumentPHIDs; - } - public function getBuildPlans() { return $this->buildPlans; } @@ -141,7 +136,6 @@ final class HeraldDifferentialRevisionAdapter return array_merge( array( self::ACTION_APPLY_BUILD_PLANS, - self::ACTION_REQUIRE_SIGNATURE, ), parent::getActions($rule_type)); case HeraldRuleTypeConfig::RULE_TYPE_PERSONAL: @@ -168,15 +162,6 @@ final class HeraldDifferentialRevisionAdapter true, pht('Applied build plans.')); break; - case self::ACTION_REQUIRE_SIGNATURE: - foreach ($effect->getTarget() as $phid) { - $this->requiredSignatureDocumentPHIDs[] = $phid; - } - $result[] = new HeraldApplyTranscript( - $effect, - true, - pht('Required signatures.')); - break; default: $result[] = $this->applyStandardEffect($effect); break; diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index c4759a937e..9858545d5c 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -29,7 +29,6 @@ abstract class HeraldAdapter extends Phobject { const ACTION_AUDIT = 'audit'; const ACTION_APPLY_BUILD_PLANS = 'applybuildplans'; const ACTION_BLOCK = 'block'; - const ACTION_REQUIRE_SIGNATURE = 'signature'; private $contentSource; private $isNewObject; @@ -716,7 +715,6 @@ abstract class HeraldAdapter extends Phobject { $standard = array( self::ACTION_AUDIT => pht('Trigger an Audit by'), self::ACTION_APPLY_BUILD_PLANS => pht('Run build plans'), - self::ACTION_REQUIRE_SIGNATURE => pht('Require legal signatures'), self::ACTION_BLOCK => pht('Block change with message'), ); break; @@ -805,9 +803,6 @@ abstract class HeraldAdapter extends Phobject { case self::ACTION_APPLY_BUILD_PLANS: return $this->buildTokenizerFieldValue( new HarbormasterBuildPlanDatasource()); - case self::ACTION_REQUIRE_SIGNATURE: - return $this->buildTokenizerFieldValue( - new LegalpadDocumentDatasource()); case self::ACTION_BLOCK: return new HeraldTextFieldValue(); } diff --git a/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php b/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php new file mode 100644 index 0000000000..092840df68 --- /dev/null +++ b/src/applications/legalpad/herald/LegalpadRequireSignatureHeraldAction.php @@ -0,0 +1,197 @@ +getAdapter(); + $edgetype_legal = LegalpadObjectNeedsSignatureEdgeType::EDGECONST; + + $phids = array_fuse($phids); + + if (!$phids) { + $this->logEffect(self::DO_NO_TARGETS); + return; + } + + $current = $adapter->loadEdgePHIDs($edgetype_legal); + + $already = array(); + foreach ($phids as $phid) { + if (isset($current[$phid])) { + $already[] = $phid; + unset($phids[$phid]); + } + } + + if ($already) { + $this->logEffect(self::DO_ALREADY_REQUIRED, $phids); + } + + if (!$phids) { + return; + } + + $documents = id(new LegalpadDocumentQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withPHIDs($phids) + ->execute(); + $documents = mpull($documents, null, 'getPHID'); + + $invalid = array(); + foreach ($phids as $phid) { + if (empty($documents[$phid])) { + $invalid[] = $phid; + unset($documents[$phid]); + } + } + + if ($invalid) { + $this->logEffect(self::DO_INVALID, $phids); + } + + if (!$phids) { + return; + } + + $object = $adapter->getObject(); + $author_phid = $object->getAuthorPHID(); + + $signatures = id(new LegalpadDocumentSignatureQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withDocumentPHIDs($phids) + ->withSignerPHIDs(array($author_phid)) + ->execute(); + $signatures = mpull($signatures, null, 'getDocumentPHID'); + + $signed = array(); + foreach ($phids as $phid) { + if (isset($signatures[$phid])) { + $signed[] = $phid; + unset($phids[$phid]); + } + } + + if ($signed) { + $this->logEffect(self::DO_SIGNED, $phids); + } + + if (!$phids) { + return; + } + + $xaction = $adapter->newTransaction() + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue('edge:type', $edgetype_legal) + ->setNewValue( + array( + '+' => $phids, + )); + + $adapter->queueTransaction($xaction); + + $this->logEffect(self::DO_REQUIRED, $phids); + } + + protected function getActionEffectMap() { + return array( + self::DO_NO_TARGETS => array( + 'icon' => 'fa-ban', + 'color' => 'grey', + 'name' => pht('No Targets'), + ), + self::DO_INVALID => array( + 'icon' => 'fa-ban', + 'color' => 'red', + 'name' => pht('Invalid Targets'), + ), + self::DO_ALREADY_REQUIRED => array( + 'icon' => 'fa-terminal', + 'color' => 'grey', + 'name' => pht('Already Required'), + ), + self::DO_SIGNED => array( + 'icon' => 'fa-terminal', + 'color' => 'green', + 'name' => pht('Already Signed'), + ), + self::DO_REQUIRED => array( + 'icon' => 'fa-terminal', + 'color' => 'green', + 'name' => pht('Required Signature'), + ), + ); + } + + public function renderActionEffectDescription($type, $data) { + switch ($type) { + case self::DO_NO_TARGETS: + return pht('Rule lists no targets.'); + case self::DO_INVALID: + return pht( + '%s document(s) are not valid: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_ALREADY_REQUIRED: + return pht( + '%s document signature(s) are already required: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_SIGNED: + return pht( + '%s document(s) are already signed: %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + case self::DO_REQUIRED: + return pht( + 'Required %s signature(s): %s.', + new PhutilNumber(count($data)), + $this->renderHandleList($data)); + } + } + + public function getHeraldActionName() { + return pht('Require signatures'); + } + + public function supportsRuleType($rule_type) { + return ($rule_type != HeraldRuleTypeConfig::RULE_TYPE_PERSONAL); + } + + public function applyEffect($object, HeraldEffect $effect) { + return $this->applyRequire($effect->getTarget()); + } + + public function getHeraldActionStandardType() { + return self::STANDARD_PHID_LIST; + } + + protected function getDatasource() { + return new LegalpadDocumentDatasource(); + } + + public function renderActionDescription($value) { + return pht( + 'Require document signatures: %s.', + $this->renderHandleList($value)); + } +} diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index 6284ed7a97..00bc82305c 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1333,6 +1333,11 @@ final class PhabricatorUSEnglishTranslation 'Added blocking reviewers: %2$s.', ), + 'Required %s signature(s): %s.' => array( + 'Required a signature: %2$s.', + 'Required signatures: %2$s.', + ), + ); }