From b6ba75d991df4babeb6e6bdc6cdd9eeabe8741f2 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 11 Sep 2018 10:14:56 -0700 Subject: [PATCH] Add a Phriction hint when a draft exists, and fix some draft editing bugs Summary: Depends on D19662. Ref T13077. See PHI840. - If you're looking at the published version of a document, but a draft version exists and you can edit it, add a hint/link. - Fix an issue where the "draft" transaction would complain when you created a document since the initial content is empty and no "draft" transaction is adding any content. Test Plan: Created new documents, viewed documents with current published versions and unpublished drafts. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13077 Differential Revision: https://secure.phabricator.com/D19663 --- .../PhrictionDocumentController.php | 29 +++++++++++++++++ .../controller/PhrictionEditController.php | 32 ++++++++++++------- .../PhrictionDocumentContentTransaction.php | 16 ++++++++++ .../PhrictionDocumentDraftTransaction.php | 21 ++++++++++++ .../PhrictionDocumentEditTransaction.php | 13 -------- 5 files changed, 87 insertions(+), 24 deletions(-) diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index e424a7f148..1853dd1ee1 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -186,6 +186,35 @@ final class PhrictionDocumentController ); } else { $content = $document->getContent(); + + if ($content->getVersion() < $document->getMaxVersion()) { + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $document, + PhabricatorPolicyCapability::CAN_EDIT); + if ($can_edit) { + $document_uri = new PhutilURI($document->getURI()); + $draft_uri = $document_uri->alter('v', $document->getMaxVersion()); + + $draft_link = phutil_tag( + 'a', + array( + 'href' => $draft_uri, + ), + pht('View Draft Version')); + + $draft_link = phutil_tag('strong', array(), $draft_link); + + $version_note = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) + ->appendChild( + array( + pht('This document has unpublished draft changes.'), + ' ', + $draft_link, + )); + } + } } $page_title = $content->getTitle(); diff --git a/src/applications/phriction/controller/PhrictionEditController.php b/src/applications/phriction/controller/PhrictionEditController.php index 31879632de..9f59b63121 100644 --- a/src/applications/phriction/controller/PhrictionEditController.php +++ b/src/applications/phriction/controller/PhrictionEditController.php @@ -98,7 +98,11 @@ final class PhrictionEditController $is_draft_mode = ($document->getContent()->getVersion() != $max_version); if ($request->isFormPost()) { - $save_as_draft = ($is_draft_mode || $request->getExists('draft')); + if ($is_new) { + $save_as_draft = false; + } else { + $save_as_draft = ($is_draft_mode || $request->getExists('draft')); + } $title = $request->getStr('title'); $content_text = $request->getStr('content'); @@ -117,6 +121,7 @@ final class PhrictionEditController } $xactions = array(); + $xactions[] = id(new PhrictionTransaction()) ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE) ->setNewValue($title); @@ -275,17 +280,22 @@ final class PhrictionEditController ->addCancelButton($cancel_uri) ->setValue(pht('Save Draft'))); } else { - $draft_button = id(new PHUIButtonView()) - ->setTag('input') - ->setName('draft') - ->setText(pht('Save as Draft')) - ->setColor(PHUIButtonView::GREEN); + $submit = id(new AphrontFormSubmitControl()); - $form->appendControl( - id(new AphrontFormSubmitControl()) - ->addButton($draft_button) - ->addCancelButton($cancel_uri) - ->setValue($submit_button)); + if (!$is_new) { + $draft_button = id(new PHUIButtonView()) + ->setTag('input') + ->setName('draft') + ->setText(pht('Save as Draft')) + ->setColor(PHUIButtonView::GREEN); + $submit->addButton($draft_button); + } + + $submit + ->addCancelButton($cancel_uri) + ->setValue($submit_button); + + $form->appendControl($submit); } $form_box = id(new PHUIObjectBoxView()) diff --git a/src/applications/phriction/xaction/PhrictionDocumentContentTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentContentTransaction.php index de6d92aac6..f5f1305279 100644 --- a/src/applications/phriction/xaction/PhrictionDocumentContentTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentContentTransaction.php @@ -13,4 +13,20 @@ final class PhrictionDocumentContentTransaction $this->getEditor()->setShouldPublishContent($object, true); } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + // NOTE: This is slightly different from the draft validation. Here, + // we're validating that: you can't edit away a document; and you can't + // create an empty document. + + $content = $object->getContent()->getContent(); + if ($this->isEmptyTextTransaction($content, $xactions)) { + $errors[] = $this->newRequiredError( + pht('Documents must have content.')); + } + + return $errors; + } + } diff --git a/src/applications/phriction/xaction/PhrictionDocumentDraftTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentDraftTransaction.php index 91a4b4ade5..b2f39485fc 100644 --- a/src/applications/phriction/xaction/PhrictionDocumentDraftTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentDraftTransaction.php @@ -11,4 +11,25 @@ final class PhrictionDocumentDraftTransaction $this->getEditor()->setShouldPublishContent($object, false); } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + // NOTE: We're only validating that you can't edit a document down to + // nothing in a draft transaction. Alone, this doesn't prevent you from + // creating a document with no content. The content transaction has + // validation for that. + + if (!$xactions) { + return $errors; + } + + $content = $object->getContent()->getContent(); + if ($this->isEmptyTextTransaction($content, $xactions)) { + $errors[] = $this->newRequiredError( + pht('Documents must have content.')); + } + + return $errors; + } + } diff --git a/src/applications/phriction/xaction/PhrictionDocumentEditTransaction.php b/src/applications/phriction/xaction/PhrictionDocumentEditTransaction.php index 551e8c8c2a..3130658311 100644 --- a/src/applications/phriction/xaction/PhrictionDocumentEditTransaction.php +++ b/src/applications/phriction/xaction/PhrictionDocumentEditTransaction.php @@ -79,17 +79,4 @@ abstract class PhrictionDocumentEditTransaction return $changes; } - public function validateTransactions($object, array $xactions) { - $errors = array(); - - $content = $object->getContent()->getContent(); - if ($this->isEmptyTextTransaction($content, $xactions)) { - $errors[] = $this->newRequiredError( - pht('Documents must have content.')); - } - - return $errors; - } - - }