Page MenuHomePhabricator
Files F14767784

D8676.diff
No OneTemporary
Actions

D8676.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -622,6 +622,7 @@
'DoorkeeperFeedWorkerAsana' => 'applications/doorkeeper/worker/DoorkeeperFeedWorkerAsana.php',
'DoorkeeperFeedWorkerJIRA' => 'applications/doorkeeper/worker/DoorkeeperFeedWorkerJIRA.php',
'DoorkeeperImportEngine' => 'applications/doorkeeper/engine/DoorkeeperImportEngine.php',
+ 'DoorkeeperMissingLinkException' => 'applications/doorkeeper/exception/DoorkeeperMissingLinkException.php',
'DoorkeeperObjectRef' => 'applications/doorkeeper/engine/DoorkeeperObjectRef.php',
'DoorkeeperRemarkupRule' => 'applications/doorkeeper/remarkup/DoorkeeperRemarkupRule.php',
'DoorkeeperRemarkupRuleAsana' => 'applications/doorkeeper/remarkup/DoorkeeperRemarkupRuleAsana.php',
@@ -3196,6 +3197,7 @@
'DoorkeeperFeedWorkerAsana' => 'DoorkeeperFeedWorker',
'DoorkeeperFeedWorkerJIRA' => 'DoorkeeperFeedWorker',
'DoorkeeperImportEngine' => 'Phobject',
+ 'DoorkeeperMissingLinkException' => 'Exception',
'DoorkeeperObjectRef' => 'Phobject',
'DoorkeeperRemarkupRule' => 'PhutilRemarkupRule',
'DoorkeeperRemarkupRuleAsana' => 'DoorkeeperRemarkupRule',
diff --git a/src/applications/differential/customfield/DifferentialJIRAIssuesField.php b/src/applications/differential/customfield/DifferentialJIRAIssuesField.php
--- a/src/applications/differential/customfield/DifferentialJIRAIssuesField.php
+++ b/src/applications/differential/customfield/DifferentialJIRAIssuesField.php
@@ -116,11 +116,11 @@
}
public function getOldValueForApplicationTransactions() {
- return nonempty($this->getValue(), array());
+ return array_unique(nonempty($this->getValue(), array()));
}
public function getNewValueForApplicationTransactions() {
- return nonempty($this->getValue(), array());
+ return array_unique(nonempty($this->getValue(), array()));
}
public function validateApplicationTransactions(
@@ -137,12 +137,36 @@
$transaction = null;
foreach ($xactions as $xaction) {
- $value = $xaction->getNewValue();
+ $old = $xaction->getOldValue();
+ $new = $xaction->getNewValue();
- $refs = id(new DoorkeeperImportEngine())
- ->setViewer($this->getViewer())
- ->setRefs($this->buildDoorkeeperRefs($value))
- ->execute();
+ $add = array_diff($new, $old);
+ if (!$add) {
+ continue;
+ }
+
+ // Only check that the actor can see newly added JIRA refs. You're
+ // allowed to remove refs or make no-op changes even if you aren't
+ // linked to JIRA.
+
+ try {
+ $refs = id(new DoorkeeperImportEngine())
+ ->setViewer($this->getViewer())
+ ->setRefs($this->buildDoorkeeperRefs($add))
+ ->setThrowOnMissingLink(true)
+ ->execute();
+ } catch (DoorkeeperMissingLinkException $ex) {
+ $this->error = pht('Not Linked');
+ $errors[] = new PhabricatorApplicationTransactionValidationError(
+ $type,
+ pht('Not Linked'),
+ pht(
+ 'You can not add JIRA issues (%s) to this revision because your '.
+ 'Phabricator account is not linked to a JIRA account.',
+ implode(', ', $add)),
+ $xaction);
+ continue;
+ }
$bad = array();
foreach ($refs as $ref) {
@@ -155,7 +179,7 @@
$bad = implode(', ', $bad);
$this->error = pht('Invalid');
- $error = new PhabricatorApplicationTransactionValidationError(
+ $errors[] = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Invalid'),
pht(
@@ -163,7 +187,6 @@
"you may not have permission to view them: %s",
$bad),
$xaction);
- $errors[] = $error;
}
}
diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridge.php b/src/applications/doorkeeper/bridge/DoorkeeperBridge.php
--- a/src/applications/doorkeeper/bridge/DoorkeeperBridge.php
+++ b/src/applications/doorkeeper/bridge/DoorkeeperBridge.php
@@ -3,6 +3,12 @@
abstract class DoorkeeperBridge extends Phobject {
private $viewer;
+ private $throwOnMissingLink;
+
+ public function setThrowOnMissingLink($throw_on_missing_link) {
+ $this->throwOnMissingLink = $throw_on_missing_link;
+ return $this;
+ }
final public function setViewer($viewer) {
$this->viewer = $viewer;
@@ -21,6 +27,15 @@
abstract public function pullRefs(array $refs);
public function fillObjectFromData(DoorkeeperExternalObject $obj, $result) {
+ return;
+ }
+
+ public function didFailOnMissingLink() {
+ if ($this->throwOnMissingLink) {
+ throw new DoorkeeperMissingLinkException();
+ }
+
+ return null;
}
}
diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php
--- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php
+++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php
@@ -40,7 +40,7 @@
->execute();
if (!$accounts) {
- return;
+ return $this->didFailOnMissingLink();
}
// TODO: If the user has several linked Asana accounts, we just pick the
diff --git a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php
--- a/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php
+++ b/src/applications/doorkeeper/bridge/DoorkeeperBridgeJIRA.php
@@ -34,7 +34,7 @@
->execute();
if (!$accounts) {
- return;
+ return $this->didFailOnMissingLink();
}
// TODO: When we support multiple JIRA instances, we need to disambiguate
diff --git a/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php b/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php
--- a/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php
+++ b/src/applications/doorkeeper/engine/DoorkeeperImportEngine.php
@@ -6,6 +6,7 @@
private $refs = array();
private $phids = array();
private $localOnly;
+ private $throwOnMissingLink;
public function setViewer(PhabricatorUser $viewer) {
$this->viewer = $viewer;
@@ -36,6 +37,16 @@
return $this;
}
+
+ /**
+ * Configure behavior if remote refs can not be retrieved because an
+ * authentication link is missing.
+ */
+ public function setThrowOnMissingLink($throw) {
+ $this->throwOnMissingLink = $throw;
+ return $this;
+ }
+
public function execute() {
$refs = $this->getRefs();
$viewer = $this->getViewer();
@@ -86,6 +97,7 @@
unset($bridges[$key]);
}
$bridge->setViewer($viewer);
+ $bridge->setThrowOnMissingLink($this->throwOnMissingLink);
}
$working_set = $refs;
diff --git a/src/applications/doorkeeper/exception/DoorkeeperMissingLinkException.php b/src/applications/doorkeeper/exception/DoorkeeperMissingLinkException.php
new file mode 100644
--- /dev/null
+++ b/src/applications/doorkeeper/exception/DoorkeeperMissingLinkException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class DoorkeeperMissingLinkException extends Exception {
+
+}

File Metadata

Mime Type
text/plain
Expires
Fri, Jan 24, 6:16 PM (20 h, 38 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7042404
Default Alt Text
D8676.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility