Commit 67da18e3 authored by epriestley's avatar epriestley
Browse files

When users submit "editing" inlines, warn them that their inlines will be saved

Summary: Ref T13513. This slightly expands the existing-but-hacky "warning" workflow to cover both "mentions on draft" and "submitting inlines being edited".

Test Plan:
  - Submitted changes to a revision with mentions on a draft, inlines being edited, both, and neither.
  - Got sensible warnings in the cases where warnings were appropriate.

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21191
parent 468aabd4
......@@ -9,7 +9,7 @@ return array(
'names' => array(
'conpherence.pkg.css' => '0e3cf785',
'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => 'f5ebbd7d',
'core.pkg.css' => '1b80c45d',
'core.pkg.js' => '632fb8f5',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => '2d70b7b9',
......@@ -25,7 +25,7 @@ return array(
'rsrc/audio/basic/ting.mp3' => 'a6b6540e',
'rsrc/css/aphront/aphront-bars.css' => '4a327b4a',
'rsrc/css/aphront/dark-console.css' => '7f06cda2',
'rsrc/css/aphront/dialog-view.css' => '874f5c06',
'rsrc/css/aphront/dialog-view.css' => '6f4ea703',
'rsrc/css/aphront/list-filter-view.css' => 'feb64255',
'rsrc/css/aphront/multi-column.css' => 'fbc00ba3',
'rsrc/css/aphront/notification.css' => '30240bd2',
......@@ -536,7 +536,7 @@ return array(
'almanac-css' => '2e050f4f',
'aphront-bars' => '4a327b4a',
'aphront-dark-console-css' => '7f06cda2',
'aphront-dialog-view-css' => '874f5c06',
'aphront-dialog-view-css' => '6f4ea703',
'aphront-list-filter-view-css' => 'feb64255',
'aphront-multi-column-view-css' => 'fbc00ba3',
'aphront-panel-view-css' => '46923d46',
......
......@@ -4994,6 +4994,7 @@ phutil_register_library_map(array(
'PhabricatorTransactionChange' => 'applications/transactions/data/PhabricatorTransactionChange.php',
'PhabricatorTransactionFactEngine' => 'applications/fact/engine/PhabricatorTransactionFactEngine.php',
'PhabricatorTransactionRemarkupChange' => 'applications/transactions/data/PhabricatorTransactionRemarkupChange.php',
'PhabricatorTransactionWarning' => 'applications/transactions/data/PhabricatorTransactionWarning.php',
'PhabricatorTransactions' => 'applications/transactions/constants/PhabricatorTransactions.php',
'PhabricatorTransactionsApplication' => 'applications/transactions/application/PhabricatorTransactionsApplication.php',
'PhabricatorTransactionsDestructionEngineExtension' => 'applications/transactions/engineextension/PhabricatorTransactionsDestructionEngineExtension.php',
......@@ -11768,6 +11769,7 @@ phutil_register_library_map(array(
'PhabricatorTransactionChange' => 'Phobject',
'PhabricatorTransactionFactEngine' => 'PhabricatorFactEngine',
'PhabricatorTransactionRemarkupChange' => 'PhabricatorTransactionChange',
'PhabricatorTransactionWarning' => 'Phobject',
'PhabricatorTransactions' => 'Phobject',
'PhabricatorTransactionsApplication' => 'PhabricatorApplication',
'PhabricatorTransactionsDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
......@@ -578,5 +578,45 @@ final class DifferentialTransaction
return $body;
}
public function newWarningForTransactions($object, array $xactions) {
$warning = new PhabricatorTransactionWarning();
switch ($this->getTransactionType()) {
case self::TYPE_INLINE:
$warning->setTitleText(pht('Warning: Editing Inlines'));
$warning->setContinueActionText(pht('Save Inlines and Continue'));
$count = phutil_count($xactions);
$body = array();
$body[] = pht(
'You are currently editing %s inline comment(s) on this '.
'revision.',
$count);
$body[] = pht(
'These %s inline comment(s) will be saved and published.',
$count);
$warning->setWarningParagraphs($body);
break;
case PhabricatorTransactions::TYPE_SUBSCRIBERS:
$warning->setTitleText(pht('Warning: Draft Revision'));
$warning->setContinueActionText(pht('Tell No One'));
$body = array();
$body[] = pht(
'This is a draft revision that will not publish any '.
'notifications until the author requests review.');
$body[] = pht('Mentioned or subscribed users will not be notified.');
$warning->setWarningParagraphs($body);
break;
}
return $warning;
}
}
<?php
final class PhabricatorTransactionWarning
extends Phobject {
private $titleText;
private $continueActionText;
private $cancelActionText;
private $warningParagraphs = array();
public function setTitleText($title_text) {
$this->titleText = $title_text;
return $this;
}
public function getTitleText() {
return $this->titleText;
}
public function setContinueActionText($continue_action_text) {
$this->continueActionText = $continue_action_text;
return $this;
}
public function getContinueActionText() {
return $this->continueActionText;
}
public function setCancelActionText($cancel_action_text) {
$this->cancelActionText = $cancel_action_text;
return $this;
}
public function getCancelActionText() {
return $this->cancelActionText;
}
public function setWarningParagraphs(array $warning_paragraphs) {
$this->warningParagraphs = $warning_paragraphs;
return $this;
}
public function getWarningParagraphs() {
return $this->warningParagraphs;
}
}
......@@ -2032,6 +2032,7 @@ abstract class PhabricatorEditEngine
->setException($ex);
} catch (PhabricatorApplicationTransactionWarningException $ex) {
return id(new PhabricatorApplicationTransactionWarningResponse())
->setObject($object)
->setCancelURI($view_uri)
->setException($ex);
}
......
......@@ -4975,11 +4975,18 @@ abstract class PhabricatorApplicationTransactionEditor
return false;
}
$type = $xaction->getTransactionType();
// TODO: This doesn't warn for inlines in Audit, even though they have
// the same overall workflow.
if ($type === DifferentialTransaction::TYPE_INLINE) {
return (bool)$xaction->getComment()->getAttribute('editing', false);
}
if (!$object->isDraft()) {
return false;
}
$type = $xaction->getTransactionType();
if ($type != PhabricatorTransactions::TYPE_SUBSCRIBERS) {
return false;
}
......
......@@ -10,4 +10,8 @@ final class PhabricatorApplicationTransactionWarningException
parent::__construct();
}
public function getTransactions() {
return $this->xactions;
}
}
......@@ -4,9 +4,19 @@ final class PhabricatorApplicationTransactionWarningResponse
extends AphrontProxyResponse {
private $viewer;
private $object;
private $exception;
private $cancelURI;
public function setObject($object) {
$this->object = $object;
return $this;
}
public function getObject() {
return $this->object;
}
public function setCancelURI($cancel_uri) {
$this->cancelURI = $cancel_uri;
return $this;
......@@ -18,39 +28,115 @@ final class PhabricatorApplicationTransactionWarningResponse
return $this;
}
public function getException() {
return $this->exception;
}
protected function buildProxy() {
return new AphrontDialogResponse();
}
public function reduceProxyResponse() {
$request = $this->getRequest();
$viewer = $request->getViewer();
$object = $this->getObject();
$title = pht('Warning: Unexpected Effects');
$xactions = $this->getException()->getTransactions();
$xaction_groups = mgroup($xactions, 'getTransactionType');
$head = pht(
'This is a draft revision that will not publish any notifications '.
'until the author requests review.');
$tail = pht(
'Mentioned or subscribed users will not be notified.');
$warnings = array();
foreach ($xaction_groups as $xaction_group) {
$xaction = head($xaction_group);
$continue = pht('Tell No One');
$warning = $xaction->newWarningForTransactions(
$object,
$xaction_group);
if (!($warning instanceof PhabricatorTransactionWarning)) {
throw new Exception(
pht(
'Expected "newTransactionWarning()" to return an object of '.
'class "PhabricatorTransactionWarning", got something else '.
'("%s") from transaction of class "%s".',
phutil_describe_type($warning),
get_class($xaction)));
}
$warnings[] = $warning;
}
$dialog = id(new AphrontDialogView())
->setViewer($request->getViewer())
->setTitle($title);
->setViewer($viewer);
$last_key = last_key($warnings);
foreach ($warnings as $warning_key => $warning) {
$paragraphs = $warning->getWarningParagraphs();
foreach ($paragraphs as $paragraph) {
$dialog->appendParagraph($paragraph);
}
if ($warning_key !== $last_key) {
$dialog->appendChild(phutil_tag('hr'));
}
}
$title_texts = array();
$continue_texts = array();
$cancel_texts = array();
foreach ($warnings as $warning) {
$title_text = $warning->getTitleText();
if ($title_text !== null) {
$title_texts[] = $title_text;
}
$dialog->appendParagraph($head);
$dialog->appendParagraph($tail);
$continue_text = $warning->getContinueActionText();
if ($continue_text !== null) {
$continue_texts[] = $continue_text;
}
$cancel_text = $warning->getCancelActionText();
if ($cancel_text !== null) {
$cancel_texts[] = $cancel_text;
}
}
$title_texts = array_unique($title_texts);
if (count($title_texts) === 1) {
$title = head($title_texts);
} else {
$title = pht('Warnings');
}
$continue_texts = array_unique($continue_texts);
if (count($continue_texts) === 1) {
$continue_action = head($continue_texts);
} else {
$continue_action = pht('Continue');
}
$cancel_texts = array_unique($cancel_texts);
if (count($cancel_texts) === 1) {
$cancel_action = head($cancel_texts);
} else {
$cancel_action = null;
}
$dialog
->setTitle($title)
->addSubmitButton($continue_action);
if ($cancel_action === null) {
$dialog->addCancelButton($this->cancelURI);
} else {
$dialog->addCancelButton($this->cancelURI, $cancel_action);
}
$passthrough = $request->getPassthroughRequestParameters();
foreach ($passthrough as $key => $value) {
$dialog->addHiddenInput($key, $value);
}
$dialog
->addHiddenInput('editEngine.warnings', 1)
->addSubmitButton($continue)
->addCancelButton($this->cancelURI);
$dialog->addHiddenInput('editEngine.warnings', 1);
return $this->getProxy()->setDialog($dialog);
}
......
......@@ -196,4 +196,10 @@ abstract class PhabricatorModularTransaction
return $this->getTransactionImplementation()->newRemarkupChanges();
}
/* final */ public function newWarningForTransactions(
$object,
array $xactions) {
throw new PhutilMethodNotImplementedException();
}
}
......@@ -1723,6 +1723,17 @@ final class PhabricatorUSEnglishTranslation
'View All %d Subscribers',
),
'You are currently editing %s inline comment(s) on this '.
'revision.' => array(
'You are currently editing an inline comment on this revision.',
'You are currently editing %s inline comments on this revision.',
),
'These %s inline comment(s) will be saved and published.' => array(
'This inline comment will be saved and published.',
'These inline comments will be saved and published.',
),
);
}
......
......@@ -192,3 +192,8 @@
.aphront-dialog-tab-group .aphront-dialog-body {
padding: 0 12px;
}
.aphront-dialog-body > hr {
background: {$thinblueborder};
margin: 24px 12px;
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment