Page MenuHomePhabricator

D15427.id37188.diff
No OneTemporary

D15427.id37188.diff

diff --git a/src/applications/differential/query/DifferentialDiffQuery.php b/src/applications/differential/query/DifferentialDiffQuery.php
--- a/src/applications/differential/query/DifferentialDiffQuery.php
+++ b/src/applications/differential/query/DifferentialDiffQuery.php
@@ -6,7 +6,9 @@
private $ids;
private $phids;
private $revisionIDs;
+
private $needChangesets = false;
+ private $needProperties;
public function withIDs(array $ids) {
$this->ids = $ids;
@@ -28,19 +30,17 @@
return $this;
}
+ public function needProperties($need_properties) {
+ $this->needProperties = $need_properties;
+ return $this;
+ }
+
+ public function newResultObject() {
+ return new DifferentialDiff();
+ }
+
protected function loadPage() {
- $table = new DifferentialDiff();
- $conn_r = $table->establishConnection('r');
-
- $data = queryfx_all(
- $conn_r,
- 'SELECT * FROM %T %Q %Q %Q',
- $table->getTableName(),
- $this->buildWhereClause($conn_r),
- $this->buildOrderClause($conn_r),
- $this->buildLimitClause($conn_r));
-
- return $table->loadAllFromArray($data);
+ return $this->loadStandardPage($this->newResultObject());
}
protected function willFilterPage(array $diffs) {
@@ -76,6 +76,23 @@
return $diffs;
}
+ protected function didFilterPage(array $diffs) {
+ if ($this->needProperties) {
+ $properties = id(new DifferentialDiffProperty())->loadAllWhere(
+ 'diffID IN (%Ld)',
+ mpull($diffs, 'getID'));
+
+ $properties = mgroup($properties, 'getDiffID');
+ foreach ($diffs as $diff) {
+ $map = idx($properties, $diff->getID(), array());
+ $map = mpull($map, 'getData', 'getName');
+ $diff->attachDiffProperties($map);
+ }
+ }
+
+ return $diffs;
+ }
+
private function loadChangesets(array $diffs) {
id(new DifferentialChangesetQuery())
->setViewer($this->getViewer())
@@ -88,32 +105,31 @@
return $diffs;
}
- protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
- $where = array();
+ protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
+ $where = parent::buildWhereClauseParts($conn);
if ($this->ids) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'id IN (%Ld)',
$this->ids);
}
if ($this->phids) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'phid IN (%Ls)',
$this->phids);
}
if ($this->revisionIDs) {
$where[] = qsprintf(
- $conn_r,
+ $conn,
'revisionID IN (%Ld)',
$this->revisionIDs);
}
- $where[] = $this->buildPagingClause($conn_r);
- return $this->formatWhereClause($where);
+ return $where;
}
public function getQueryApplicationClass() {
diff --git a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
--- a/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
+++ b/src/applications/drydock/operation/DrydockLandRepositoryOperation.php
@@ -248,6 +248,29 @@
);
}
+ // Check if this diff was pushed to a staging area.
+ $diff = id(new DifferentialDiffQuery())
+ ->setViewer($viewer)
+ ->withIDs(array($revision->getActiveDiff()->getID()))
+ ->needProperties(true)
+ ->executeOne();
+
+ // Older diffs won't have this property. They may still have been pushed.
+ // At least for now, assume staging changes are present if the property
+ // is missing. This should smooth the transition to the more formal
+ // approach.
+ $has_staging = $diff->hasDiffProperty('arc.staging');
+ if ($has_staging) {
+ $staging = $diff->getProperty('arc.staging');
+ if (!is_array($staging)) {
+ $staging = array();
+ }
+ $status = idx($staging, 'status');
+ if ($status != ArcanistDiffWorkflow::STAGING_PUSHED) {
+ return $this->getBarrierToLandingFromStagingStatus($status);
+ }
+ }
+
// TODO: At some point we should allow installs to give "land reviewed
// code" permission to more users than "push any commit", because it is
// a much less powerful operation. For now, just require push so this
@@ -336,4 +359,85 @@
return null;
}
+ private function getBarrierToLandingFromStagingStatus($status) {
+ switch ($status) {
+ case ArcanistDiffWorkflow::STAGING_USER_SKIP:
+ return array(
+ 'title' => pht('Staging Area Skipped'),
+ 'body' => pht(
+ 'The diff author used the %s flag to skip pushing this change to '.
+ 'staging. Changes must be pushed to staging before they can be '.
+ 'landed from the web.',
+ phutil_tag('tt', array(), '--skip-staging')),
+ );
+ case ArcanistDiffWorkflow::STAGING_DIFF_RAW:
+ return array(
+ 'title' => pht('Raw Diff Source'),
+ 'body' => pht(
+ 'The diff was generated from a raw input source, so the change '.
+ 'could not be pushed to staging. Changes must be pushed to '.
+ 'staging before they can be landed from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNKNOWN:
+ return array(
+ 'title' => pht('Unknown Repository'),
+ 'body' => pht(
+ 'When the diff was generated, the client was not able to '.
+ 'determine which repository it belonged to, so the change '.
+ 'was not pushed to staging. Changes must be pushed to staging '.
+ 'before they can be landed from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNAVAILABLE:
+ return array(
+ 'title' => pht('Staging Unavailable'),
+ 'body' => pht(
+ 'When this diff was generated, the server was running an older '.
+ 'version of Phabricator which did not support staging areas, so '.
+ 'the change was not pushed to staging. Changes must be pushed '.
+ 'to staging before they can be landed from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNSUPPORTED:
+ return array(
+ 'title' => pht('Repository Unsupported'),
+ 'body' => pht(
+ 'When this diff was generated, the server was running an older '.
+ 'version of Phabricator which did not support staging areas for '.
+ 'this version control system, so the chagne was not pushed to '.
+ 'staging. Changes must be pushed to staging before they can be '.
+ 'landed from the web.'),
+ );
+
+ case ArcanistDiffWorkflow::STAGING_REPOSITORY_UNCONFIGURED:
+ return array(
+ 'title' => pht('Repository Unconfigured'),
+ 'body' => pht(
+ 'When this diff was generated, the repository was not configured '.
+ 'with a staging area, so the change was not pushed to staging. '.
+ 'Changes must be pushed to staging before they can be landed '.
+ 'from the web.'),
+ );
+ case ArcanistDiffWorkflow::STAGING_CLIENT_UNSUPPORTED:
+ return array(
+ 'title' => pht('Client Support Unavailable'),
+ 'body' => pht(
+ 'When this diff was generated, the client did not support '.
+ 'staging areas for this version control system, so the change '.
+ 'was not pushed to staging. Changes must be pushed to staging '.
+ 'before they can be landed from the web. Updating the client '.
+ 'may resolve this issue.'),
+ );
+ default:
+ return array(
+ 'title' => pht('Unknown Error'),
+ 'body' => pht(
+ 'When this diff was generated, it was not pushed to staging for '.
+ 'an unknown reason (the status code was "%s"). Changes must be '.
+ 'pushed to staging before they can be landed from the web. '.
+ 'The server may be running an out-of-date version of Phabricator, '.
+ 'and updating may provide more information about this error.',
+ $status),
+ );
+ }
+ }
+
}
diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php b/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildLog.php
@@ -114,6 +114,8 @@
$this->rope->append($content);
$this->flush();
+
+ return $this;
}
private function flush() {

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 14, 7:09 AM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7225973
Default Alt Text
D15427.id37188.diff (8 KB)

Event Timeline