Page MenuHomePhabricator
Files F15416149

D8475.id20128.diff
No OneTemporary
Actions

D8475.id20128.diff
View Options

diff --git a/resources/celerity/packages.php b/resources/celerity/packages.php
--- a/resources/celerity/packages.php
+++ b/resources/celerity/packages.php
@@ -138,7 +138,6 @@
'javelin-behavior-differential-populate',
'javelin-behavior-differential-show-more',
'javelin-behavior-differential-diff-radios',
- 'javelin-behavior-differential-accept-with-errors',
'javelin-behavior-differential-comment-jump',
'javelin-behavior-differential-add-reviewers-and-ccs',
'javelin-behavior-differential-keyboard-navigation',
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
@@ -399,7 +399,6 @@
'DifferentialLandingToHostedGit' => 'applications/differential/landing/DifferentialLandingToHostedGit.php',
'DifferentialLandingToHostedMercurial' => 'applications/differential/landing/DifferentialLandingToHostedMercurial.php',
'DifferentialLintField' => 'applications/differential/customfield/DifferentialLintField.php',
- 'DifferentialLintFieldSpecification' => 'applications/differential/field/specification/DifferentialLintFieldSpecification.php',
'DifferentialLintStatus' => 'applications/differential/constants/DifferentialLintStatus.php',
'DifferentialLocalCommitsView' => 'applications/differential/view/DifferentialLocalCommitsView.php',
'DifferentialMail' => 'applications/differential/mail/DifferentialMail.php',
@@ -452,7 +451,6 @@
'DifferentialTransactionQuery' => 'applications/differential/query/DifferentialTransactionQuery.php',
'DifferentialTransactionView' => 'applications/differential/view/DifferentialTransactionView.php',
'DifferentialUnitField' => 'applications/differential/customfield/DifferentialUnitField.php',
- 'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php',
'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php',
'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php',
'DifferentialViewPolicyField' => 'applications/differential/customfield/DifferentialViewPolicyField.php',
diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -336,6 +336,19 @@
$comment_form = new DifferentialAddCommentView();
$comment_form->setRevision($revision);
+ $review_warnings = array();
+ foreach ($field_list->getFields() as $field) {
+ $review_warnings[] = $field->getWarningsForDetailView();
+ }
+ $review_warnings = array_mergev($review_warnings);
+
+ if ($review_warnings) {
+ $review_warnings_panel = id(new AphrontErrorView())
+ ->setSeverity(AphrontErrorView::SEVERITY_WARNING)
+ ->setErrors($review_warnings);
+ $comment_form->setErrorView($review_warnings_panel);
+ }
+
// TODO: Restore the ability for fields to add accept warnings.
$comment_form->setActions($this->getRevisionCommentActions($revision));
diff --git a/src/applications/differential/customfield/DifferentialCustomField.php b/src/applications/differential/customfield/DifferentialCustomField.php
--- a/src/applications/differential/customfield/DifferentialCustomField.php
+++ b/src/applications/differential/customfield/DifferentialCustomField.php
@@ -76,6 +76,12 @@
return implode(', ', $out);
}
+ public function getWarningsForDetailView() {
+ if ($this->getProxy()) {
+ return $this->getProxy()->getWarningsForDetailView();
+ }
+ return array();
+ }
/* -( Integration with Commit Messages )----------------------------------- */
diff --git a/src/applications/differential/customfield/DifferentialLintField.php b/src/applications/differential/customfield/DifferentialLintField.php
--- a/src/applications/differential/customfield/DifferentialLintField.php
+++ b/src/applications/differential/customfield/DifferentialLintField.php
@@ -233,4 +233,25 @@
return "Show Full Lint Results (".implode(', ', $show).")";
}
+
+ public function getWarningsForDetailView() {
+ $status = $this->getObject()->getActiveDiff()->getLintStatus();
+ if ($status < DifferentialLintStatus::LINT_WARN) {
+ return array();
+ }
+
+ $warnings = array();
+ if ($status == DifferentialLintStatus::LINT_SKIP) {
+ $warnings[] = pht(
+ 'Lint was skipped when generating these changes.');
+ } else if ($status == DifferentialLintStatus::LINT_POSTPONED) {
+ $warnings[] = pht(
+ 'Background linting has not finished executing on these changes.');
+ } else {
+ $warnings[] = pht('These changes have lint problems.');
+ }
+
+ return $warnings;
+ }
+
}
diff --git a/src/applications/differential/customfield/DifferentialUnitField.php b/src/applications/differential/customfield/DifferentialUnitField.php
--- a/src/applications/differential/customfield/DifferentialUnitField.php
+++ b/src/applications/differential/customfield/DifferentialUnitField.php
@@ -203,4 +203,24 @@
return "Show Full Unit Results (".implode(', ', $show).")";
}
+ public function getWarningsForDetailView() {
+ $status = $this->getObject()->getActiveDiff()->getUnitStatus();
+
+ $warnings = array();
+ if ($status < DifferentialUnitStatus::UNIT_WARN) {
+ // Don't show any warnings.
+ } else if ($status == DifferentialUnitStatus::UNIT_POSTPONED) {
+ $warnings[] = pht(
+ 'Background tests have not finished executing on these changes.');
+ } else if ($status == DifferentialUnitStatus::UNIT_SKIP) {
+ $warnings[] = pht(
+ 'Unit tests were skipped when generating these changes.');
+ } else {
+ $warnings[] = pht('These changes have unit test problems.');
+ }
+
+ return $warnings;
+ }
+
+
}
diff --git a/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php b/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php
deleted file mode 100644
--- a/src/applications/differential/field/specification/DifferentialLintFieldSpecification.php
+++ /dev/null
@@ -1,42 +0,0 @@
-<?php
-
-final class DifferentialLintFieldSpecification {
-
- public function renderWarningBoxForRevisionAccept() {
- $status = $this->getDiff()->getLintStatus();
- if ($status < DifferentialLintStatus::LINT_WARN) {
- return null;
- }
-
- $severity = AphrontErrorView::SEVERITY_ERROR;
- $titles = array(
- DifferentialLintStatus::LINT_WARN => 'Lint Warning',
- DifferentialLintStatus::LINT_FAIL => 'Lint Failure',
- DifferentialLintStatus::LINT_SKIP => 'Lint Skipped',
- DifferentialLintStatus::LINT_POSTPONED => 'Lint Postponed',
- );
-
- if ($status == DifferentialLintStatus::LINT_SKIP) {
- $content =
- "This diff was created without running lint. Make sure you are ".
- "OK with that before you accept this diff.";
-
- } else if ($status == DifferentialLintStatus::LINT_POSTPONED) {
- $severity = AphrontErrorView::SEVERITY_WARNING;
- $content =
- "Postponed linters didn't finish yet. Make sure you are OK with ".
- "that before you accept this diff.";
-
- } else {
- $content =
- "This diff has Lint Problems. Make sure you are OK with them ".
- "before you accept this diff.";
- }
-
- return id(new AphrontErrorView())
- ->setSeverity($severity)
- ->appendChild(phutil_tag('p', array(), $content))
- ->setTitle(idx($titles, $status, 'Warning'));
- }
-
-}
diff --git a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php
deleted file mode 100644
--- a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php
+++ /dev/null
@@ -1,38 +0,0 @@
-<?php
-
-final class DifferentialUnitFieldSpecification {
-
- public function renderWarningBoxForRevisionAccept() {
- $diff = $this->getDiff();
- $unit_warning = null;
- if ($diff->getUnitStatus() >= DifferentialUnitStatus::UNIT_WARN) {
- $titles =
- array(
- DifferentialUnitStatus::UNIT_WARN => 'Unit Tests Warning',
- DifferentialUnitStatus::UNIT_FAIL => 'Unit Tests Failure',
- DifferentialUnitStatus::UNIT_SKIP => 'Unit Tests Skipped',
- DifferentialUnitStatus::UNIT_POSTPONED => 'Unit Tests Postponed'
- );
- if ($diff->getUnitStatus() == DifferentialUnitStatus::UNIT_POSTPONED) {
- $content =
- "This diff has postponed unit tests. The results should be ".
- "coming in soon. You should probably wait for them before accepting ".
- "this diff.";
- } else if ($diff->getUnitStatus() == DifferentialUnitStatus::UNIT_SKIP) {
- $content =
- "Unit tests were skipped when this diff was created. Make sure ".
- "you are OK with that before you accept this diff.";
- } else {
- $content =
- "This diff has Unit Test Problems. Make sure you are OK with ".
- "them before you accept this diff.";
- }
- $unit_warning = id(new AphrontErrorView())
- ->setSeverity(AphrontErrorView::SEVERITY_ERROR)
- ->appendChild(phutil_tag('p', array(), $content))
- ->setTitle(idx($titles, $diff->getUnitStatus(), 'Warning'));
- }
- return $unit_warning;
- }
-
-}
diff --git a/src/applications/differential/view/DifferentialAddCommentView.php b/src/applications/differential/view/DifferentialAddCommentView.php
--- a/src/applications/differential/view/DifferentialAddCommentView.php
+++ b/src/applications/differential/view/DifferentialAddCommentView.php
@@ -8,6 +8,16 @@
private $draft;
private $reviewers = array();
private $ccs = array();
+ private $errorView;
+
+ public function setErrorView(AphrontErrorView $error_view) {
+ $this->errorView = $error_view;
+ return $this;
+ }
+
+ public function getErrorView() {
+ return $this->errorView;
+ }
public function setRevision($revision) {
$this->revision = $revision;
@@ -129,15 +139,6 @@
));
$diff = $revision->loadActiveDiff();
- $warnings = array();
-
- Javelin::initBehavior(
- 'differential-accept-with-errors',
- array(
- 'select' => 'comment-action',
- 'warnings' => 'warnings',
- ));
-
$rev_id = $revision->getID();
Javelin::initBehavior(
@@ -156,13 +157,6 @@
'inline' => 'inline-comment-preview',
));
- $warning_container = array();
- foreach ($warnings as $warning) {
- if ($warning) {
- $warning_container[] = $warning->render();
- }
- }
-
$header = id(new PHUIHeaderView())
->setHeader($is_serious ? pht('Add Comment') : pht('Leap Into Action'));
@@ -170,8 +164,6 @@
->setAnchorName('comment')
->setNavigationMarker(true);
- $warn = phutil_tag('div', array('id' => 'warnings'), $warning_container);
-
$loading = phutil_tag(
'span',
array('class' => 'aphront-panel-preview-loading-text'),
@@ -188,9 +180,12 @@
$comment_box = id(new PHUIObjectBoxView())
->setHeader($header)
->appendChild($anchor)
- ->appendChild($warn)
->appendChild($form);
+ if ($this->errorView) {
+ $comment_box->setErrorView($this->errorView);
+ }
+
return array($comment_box, $preview);
}
}
diff --git a/webroot/rsrc/js/application/differential/behavior-accept-with-errors.js b/webroot/rsrc/js/application/differential/behavior-accept-with-errors.js
deleted file mode 100644
--- a/webroot/rsrc/js/application/differential/behavior-accept-with-errors.js
+++ /dev/null
@@ -1,25 +0,0 @@
-/**
- * @provides javelin-behavior-differential-accept-with-errors
- * @requires javelin-behavior
- * javelin-dom
- */
-
-JX.behavior('differential-accept-with-errors', function(config) {
- if (config.warnings) {
- toggleWarning();
- JX.DOM.listen(
- JX.$(config.select),
- 'change',
- null,
- toggleWarning);
- }
-
- function toggleWarning() {
- if (JX.$(config.select).value == 'accept') {
- JX.DOM.show(JX.$(config.warnings));
- } else {
- JX.DOM.hide(JX.$(config.warnings));
- }
- }
-
-});

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 21, 9:37 AM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7715290
Default Alt Text
D8475.id20128.diff (12 KB)

Event Timeline

Phabricator · Built by Phacility