diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ '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 @@ '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 @@ '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', 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 @@ -4994,6 +4994,7 @@ '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 @@ 'PhabricatorTransactionChange' => 'Phobject', 'PhabricatorTransactionFactEngine' => 'PhabricatorFactEngine', 'PhabricatorTransactionRemarkupChange' => 'PhabricatorTransactionChange', + 'PhabricatorTransactionWarning' => 'Phobject', 'PhabricatorTransactions' => 'Phobject', 'PhabricatorTransactionsApplication' => 'PhabricatorApplication', 'PhabricatorTransactionsDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension', diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -578,5 +578,45 @@ 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; + } + } diff --git a/src/applications/transactions/data/PhabricatorTransactionWarning.php b/src/applications/transactions/data/PhabricatorTransactionWarning.php new file mode 100644 --- /dev/null +++ b/src/applications/transactions/data/PhabricatorTransactionWarning.php @@ -0,0 +1,47 @@ +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; + } + +} diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -2032,6 +2032,7 @@ ->setException($ex); } catch (PhabricatorApplicationTransactionWarningException $ex) { return id(new PhabricatorApplicationTransactionWarningResponse()) + ->setObject($object) ->setCancelURI($view_uri) ->setException($ex); } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -4975,11 +4975,18 @@ 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; } diff --git a/src/applications/transactions/exception/PhabricatorApplicationTransactionWarningException.php b/src/applications/transactions/exception/PhabricatorApplicationTransactionWarningException.php --- a/src/applications/transactions/exception/PhabricatorApplicationTransactionWarningException.php +++ b/src/applications/transactions/exception/PhabricatorApplicationTransactionWarningException.php @@ -10,4 +10,8 @@ parent::__construct(); } + public function getTransactions() { + return $this->xactions; + } + } diff --git a/src/applications/transactions/response/PhabricatorApplicationTransactionWarningResponse.php b/src/applications/transactions/response/PhabricatorApplicationTransactionWarningResponse.php --- a/src/applications/transactions/response/PhabricatorApplicationTransactionWarningResponse.php +++ b/src/applications/transactions/response/PhabricatorApplicationTransactionWarningResponse.php @@ -4,9 +4,19 @@ 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 @@ 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); } diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php --- a/src/applications/transactions/storage/PhabricatorModularTransaction.php +++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php @@ -196,4 +196,10 @@ return $this->getTransactionImplementation()->newRemarkupChanges(); } + /* final */ public function newWarningForTransactions( + $object, + array $xactions) { + throw new PhutilMethodNotImplementedException(); + } + } diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1723,6 +1723,17 @@ '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.', + ), + ); } diff --git a/webroot/rsrc/css/aphront/dialog-view.css b/webroot/rsrc/css/aphront/dialog-view.css --- a/webroot/rsrc/css/aphront/dialog-view.css +++ b/webroot/rsrc/css/aphront/dialog-view.css @@ -192,3 +192,8 @@ .aphront-dialog-tab-group .aphront-dialog-body { padding: 0 12px; } + +.aphront-dialog-body > hr { + background: {$thinblueborder}; + margin: 24px 12px; +}