diff --git a/resources/sql/patches/087.phrictiondelete.sql b/resources/sql/patches/087.phrictiondelete.sql new file mode 100644 index 0000000000..eeb377f0b3 --- /dev/null +++ b/resources/sql/patches/087.phrictiondelete.sql @@ -0,0 +1,8 @@ +ALTER TABLE phabricator_phriction.phriction_document + ADD status INT UNSIGNED NOT NULL DEFAULT 0; + +ALTER TABLE phabricator_phriction.phriction_content + ADD changeType INT UNSIGNED NOT NULL DEFAULT 0; + +ALTER TABLE phabricator_phriction.phriction_content + ADD changeRef INT UNSIGNED DEFAULT NULL; \ No newline at end of file diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8ee18b9598..bab5bcf691 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -719,15 +719,18 @@ phutil_register_library_map(array( 'PhabricatorXHProfProfileSymbolView' => 'applications/xhprof/view/symbol', 'PhabricatorXHProfProfileTopLevelView' => 'applications/xhprof/view/toplevel', 'PhrictionActionConstants' => 'applications/phriction/constants/action', + 'PhrictionChangeType' => 'applications/phriction/constants/changetype', 'PhrictionConstants' => 'applications/phriction/constants/base', 'PhrictionContent' => 'applications/phriction/storage/content', 'PhrictionController' => 'applications/phriction/controller/base', 'PhrictionDAO' => 'applications/phriction/storage/base', + 'PhrictionDeleteController' => 'applications/phriction/controller/delete', 'PhrictionDiffController' => 'applications/phriction/controller/diff', 'PhrictionDocument' => 'applications/phriction/storage/document', 'PhrictionDocumentController' => 'applications/phriction/controller/document', 'PhrictionDocumentEditor' => 'applications/phriction/editor/document', 'PhrictionDocumentPreviewController' => 'applications/phriction/controller/documentpreview', + 'PhrictionDocumentStatus' => 'applications/phriction/constants/documentstatus', 'PhrictionDocumentTestCase' => 'applications/phriction/storage/document/__tests__', 'PhrictionEditController' => 'applications/phriction/controller/edit', 'PhrictionHistoryController' => 'applications/phriction/controller/history', @@ -1340,13 +1343,16 @@ phutil_register_library_map(array( 'PhabricatorXHProfProfileSymbolView' => 'AphrontView', 'PhabricatorXHProfProfileTopLevelView' => 'AphrontView', 'PhrictionActionConstants' => 'PhrictionConstants', + 'PhrictionChangeType' => 'PhrictionConstants', 'PhrictionContent' => 'PhrictionDAO', 'PhrictionController' => 'PhabricatorController', 'PhrictionDAO' => 'PhabricatorLiskDAO', + 'PhrictionDeleteController' => 'PhrictionController', 'PhrictionDiffController' => 'PhrictionController', 'PhrictionDocument' => 'PhrictionDAO', 'PhrictionDocumentController' => 'PhrictionController', 'PhrictionDocumentPreviewController' => 'PhrictionController', + 'PhrictionDocumentStatus' => 'PhrictionConstants', 'PhrictionDocumentTestCase' => 'PhabricatorTestCase', 'PhrictionEditController' => 'PhrictionController', 'PhrictionHistoryController' => 'PhrictionController', diff --git a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php index c172bca5c9..17ece2998f 100644 --- a/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php @@ -365,6 +365,7 @@ class AphrontDefaultApplicationConfiguration 'history/(?P.+/)$' => 'PhrictionHistoryController', 'edit/(?:(?P\d+)/)?$' => 'PhrictionEditController', + 'delete/(?P\d+)/$' => 'PhrictionDeleteController', 'preview/$' => 'PhrictionDocumentPreviewController', 'diff/(?P\d+)/$' => 'PhrictionDiffController', diff --git a/src/applications/conduit/method/phriction/base/ConduitAPI_phriction_Method.php b/src/applications/conduit/method/phriction/base/ConduitAPI_phriction_Method.php index 8ed56cf690..a616d4353c 100644 --- a/src/applications/conduit/method/phriction/base/ConduitAPI_phriction_Method.php +++ b/src/applications/conduit/method/phriction/base/ConduitAPI_phriction_Method.php @@ -33,6 +33,8 @@ abstract class ConduitAPI_phriction_Method extends ConduitAPIMethod { $uri = PhrictionDocument::getSlugURI($content->getSlug()); $uri = PhabricatorEnv::getProductionURI($uri); + $doc_status = $doc->getStatus(); + return array( 'phid' => $doc->getPHID(), 'uri' => $uri, @@ -41,6 +43,7 @@ abstract class ConduitAPI_phriction_Method extends ConduitAPIMethod { 'authorPHID' => $content->getAuthorPHID(), 'title' => $content->getTitle(), 'content' => $content->getContent(), + 'status' => PhrictionDocumentStatus::getConduitConstant($doc_status), 'description' => $content->getDescription(), 'dateCreated' => $content->getDateCreated(), ); diff --git a/src/applications/conduit/method/phriction/base/__init__.php b/src/applications/conduit/method/phriction/base/__init__.php index c29bfaef8f..0778a06b4f 100644 --- a/src/applications/conduit/method/phriction/base/__init__.php +++ b/src/applications/conduit/method/phriction/base/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/conduit/method/base'); +phutil_require_module('phabricator', 'applications/phriction/constants/documentstatus'); phutil_require_module('phabricator', 'applications/phriction/storage/document'); phutil_require_module('phabricator', 'infrastructure/env'); diff --git a/src/applications/phriction/constants/action/PhrictionActionConstants.php b/src/applications/phriction/constants/action/PhrictionActionConstants.php index 90005e30c4..ef92d328b8 100644 --- a/src/applications/phriction/constants/action/PhrictionActionConstants.php +++ b/src/applications/phriction/constants/action/PhrictionActionConstants.php @@ -23,11 +23,13 @@ class PhrictionActionConstants extends PhrictionConstants { const ACTION_CREATE = 'create'; const ACTION_EDIT = 'edit'; + const ACTION_DELETE = 'delete'; public static function getActionPastTenseVerb($action) { static $map = array( self::ACTION_CREATE => 'created', self::ACTION_EDIT => 'edited', + self::ACTION_DELETE => 'deleted', ); return idx($map, $action, "brazenly {$action}'d"); diff --git a/src/applications/phriction/constants/changetype/PhrictionChangeType.php b/src/applications/phriction/constants/changetype/PhrictionChangeType.php new file mode 100644 index 0000000000..dfe400d94c --- /dev/null +++ b/src/applications/phriction/constants/changetype/PhrictionChangeType.php @@ -0,0 +1,40 @@ + 'Edit', + self::CHANGE_DELETE => 'Delete', + self::CHANGE_MOVE_HERE => 'Move Here', + self::CHANGE_MOVE_AWAY => 'Move Away', + ); + + return idx($map, $const, '???'); + } + +} diff --git a/src/applications/phriction/constants/changetype/__init__.php b/src/applications/phriction/constants/changetype/__init__.php new file mode 100644 index 0000000000..e1942d537a --- /dev/null +++ b/src/applications/phriction/constants/changetype/__init__.php @@ -0,0 +1,14 @@ + 'exists', + self::STATUS_DELETED => 'deleted', + ); + + return idx($map, $const, 'unknown'); + } + +} diff --git a/src/applications/phriction/constants/documentstatus/__init__.php b/src/applications/phriction/constants/documentstatus/__init__.php new file mode 100644 index 0000000000..f28aa9655e --- /dev/null +++ b/src/applications/phriction/constants/documentstatus/__init__.php @@ -0,0 +1,14 @@ +id = $data['id']; + } + + public function processRequest() { + + $request = $this->getRequest(); + $user = $request->getUser(); + + $document = id(new PhrictionDocument())->load($this->id); + if (!$document) { + return new Aphront404Response(); + } + + $document_uri = PhrictionDocument::getSlugURI($document->getSlug()); + + if ($request->isFormPost()) { + $editor = id(PhrictionDocumentEditor::newForSlug($document->getSlug())) + ->setUser($user) + ->delete(); + return id(new AphrontRedirectResponse())->setURI($document_uri); + } + + $dialog = id(new AphrontDialogView()) + ->setUser($user) + ->setTitle('Delete document?') + ->appendChild( + 'Really delete this document? You can recover it later by reverting '. + 'to a previous version.') + ->addSubmitButton('Delete') + ->addCancelButton($document_uri); + + return id(new AphrontDialogResponse())->setDialog($dialog); + } + +} diff --git a/src/applications/phriction/controller/delete/__init__.php b/src/applications/phriction/controller/delete/__init__.php new file mode 100644 index 0000000000..e61585e211 --- /dev/null +++ b/src/applications/phriction/controller/delete/__init__.php @@ -0,0 +1,20 @@ +getDocumentID(); $version = $content->getVersion(); + if ($content->getChangeType() == PhrictionChangeType::CHANGE_DELETE) { + // Don't show an edit/revert button for changes which deleted the content + // since it's silly. + return null; + } + if ($content->getID() == $current->getID()) { return phutil_render_tag( 'a', diff --git a/src/applications/phriction/controller/diff/__init__.php b/src/applications/phriction/controller/diff/__init__.php index 9fed3362b0..30f690b2bb 100644 --- a/src/applications/phriction/controller/diff/__init__.php +++ b/src/applications/phriction/controller/diff/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'aphront/response/ajax'); phutil_require_module('phabricator', 'applications/differential/parser/changeset'); phutil_require_module('phabricator', 'applications/phid/handle/data'); +phutil_require_module('phabricator', 'applications/phriction/constants/changetype'); phutil_require_module('phabricator', 'applications/phriction/controller/base'); phutil_require_module('phabricator', 'applications/phriction/storage/content'); phutil_require_module('phabricator', 'applications/phriction/storage/document'); diff --git a/src/applications/phriction/controller/document/PhrictionDocumentController.php b/src/applications/phriction/controller/document/PhrictionDocumentController.php index 1c8ad7ae4e..91ee048f59 100644 --- a/src/applications/phriction/controller/document/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/document/PhrictionDocumentController.php @@ -120,12 +120,28 @@ class PhrictionDocumentController $engine = PhabricatorMarkupEngine::newPhrictionMarkupEngine(); + $doc_status = $document->getStatus(); + if ($doc_status == PhrictionDocumentStatus::STATUS_EXISTS) { + $core_content = + '
'. + $engine->markupText($content->getContent()). + '
'; + } else if ($doc_status == PhrictionDocumentStatus::STATUS_DELETED) { + $notice = new AphrontErrorView(); + $notice->setSeverity(AphrontErrorView::SEVERITY_NOTICE); + $notice->setTitle('Document Deleted'); + $notice->appendChild( + 'This document has been deleted. You can edit it to put new content '. + 'here, or use history to revert to an earlier version.'); + $core_content = $notice->render(); + } else { + throw new Exception("Unknown document status '{$doc_status}'!"); + } + $page_content = '
'. $byline. - '
'. - $engine->markupText($content->getContent()). - '
'. + $core_content. '
'; $edit_button = phutil_render_tag( @@ -134,7 +150,7 @@ class PhrictionDocumentController 'href' => '/phriction/edit/'.$document->getID().'/', 'class' => 'button', ), - 'Edit Page'); + 'Edit Document'); $history_button = phutil_render_tag( 'a', array( @@ -238,12 +254,14 @@ class PhrictionDocumentController 'SELECT d.slug, d.depth, c.title FROM %T d JOIN %T c ON d.contentID = c.id WHERE d.slug LIKE %> AND d.depth IN (%d, %d) + AND d.status = %d ORDER BY d.depth, c.title LIMIT %d', $document_dao->getTableName(), $content_dao->getTableName(), ($slug == '/' ? '' : $slug), $d_child, $d_grandchild, + PhrictionDocumentStatus::STATUS_EXISTS, $limit); if (!$children) { @@ -329,12 +347,13 @@ class PhrictionDocumentController } private function renderChildDocumentLink(array $info) { + $title = nonempty($info['title'], '(Untitled Document)'); $item = phutil_render_tag( 'a', array( 'href' => PhrictionDocument::getSlugURI($info['slug']), ), - phutil_escape_html($info['title'])); + phutil_escape_html($title)); if (isset($info['empty'])) { $item = ''.$item.''; diff --git a/src/applications/phriction/controller/document/__init__.php b/src/applications/phriction/controller/document/__init__.php index 0cc26961ce..840de03791 100644 --- a/src/applications/phriction/controller/document/__init__.php +++ b/src/applications/phriction/controller/document/__init__.php @@ -11,6 +11,7 @@ phutil_require_module('phabricator', 'aphront/response/redirect'); phutil_require_module('phabricator', 'applications/markup/engine'); phutil_require_module('phabricator', 'applications/phid/handle'); phutil_require_module('phabricator', 'applications/phid/handle/data'); +phutil_require_module('phabricator', 'applications/phriction/constants/documentstatus'); phutil_require_module('phabricator', 'applications/phriction/controller/base'); phutil_require_module('phabricator', 'applications/phriction/storage/content'); phutil_require_module('phabricator', 'applications/phriction/storage/document'); diff --git a/src/applications/phriction/controller/edit/PhrictionEditController.php b/src/applications/phriction/controller/edit/PhrictionEditController.php index 452a036f16..f462eafa08 100644 --- a/src/applications/phriction/controller/edit/PhrictionEditController.php +++ b/src/applications/phriction/controller/edit/PhrictionEditController.php @@ -116,9 +116,17 @@ class PhrictionEditController if ($document->getID()) { $panel_header = 'Edit Phriction Document'; $submit_button = 'Save Changes'; + $delete_button = phutil_render_tag( + 'a', + array( + 'href' => '/phriction/delete/'.$document->getID().'/', + 'class' => 'grey button', + ), + 'Delete Document'); } else { $panel_header = 'Create New Phriction Document'; $submit_button = 'Create Document'; + $delete_button = null; } $uri = $document->getSlug(); @@ -146,12 +154,6 @@ class PhrictionEditController ->setValue($content->getTitle()) ->setError($e_title) ->setName('title')) - ->appendChild( - id(new AphrontFormTextControl()) - ->setLabel('Description') - ->setValue($content->getDescription()) - ->setError(null) - ->setName('description')) ->appendChild( id(new AphrontFormStaticControl()) ->setLabel('URI') @@ -165,6 +167,12 @@ class PhrictionEditController ->setID('document-textarea') ->setEnableDragAndDropFileUploads(true) ->setCaption($remarkup_reference)) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel('Edit Notes') + ->setValue($content->getDescription()) + ->setError(null) + ->setName('description')) ->appendChild( id(new AphrontFormSubmitControl()) ->addCancelButton($cancel_uri) @@ -175,6 +183,10 @@ class PhrictionEditController ->setHeader($panel_header) ->appendChild($form); + if ($delete_button) { + $panel->addButton($delete_button); + } + $preview_panel = '
diff --git a/src/applications/phriction/controller/history/PhrictionHistoryController.php b/src/applications/phriction/controller/history/PhrictionHistoryController.php index 75e3beb0d3..dd23997b1c 100644 --- a/src/applications/phriction/controller/history/PhrictionHistoryController.php +++ b/src/applications/phriction/controller/history/PhrictionHistoryController.php @@ -93,6 +93,9 @@ class PhrictionHistoryController 'Show Later Changes'); } + $change_type = PhrictionChangeType::getChangeTypeLabel( + $content->getChangeType()); + $rows[] = array( phabricator_date($content->getDateCreated(), $user), phabricator_time($content->getDateCreated(), $user), @@ -103,6 +106,7 @@ class PhrictionHistoryController ), 'Version '.$version), $handles[$content->getAuthorPHID()]->renderLink(), + $change_type, phutil_escape_html($content->getDescription()), $vs_previous, $vs_head, @@ -131,6 +135,7 @@ class PhrictionHistoryController 'Time', 'Version', 'Author', + 'Type', 'Description', 'Against Previous', 'Against Current', @@ -141,6 +146,7 @@ class PhrictionHistoryController 'right', 'pri', '', + '', 'wide', '', '', diff --git a/src/applications/phriction/controller/history/__init__.php b/src/applications/phriction/controller/history/__init__.php index 6c5ab29f2f..f47ab5a76f 100644 --- a/src/applications/phriction/controller/history/__init__.php +++ b/src/applications/phriction/controller/history/__init__.php @@ -8,6 +8,7 @@ phutil_require_module('phabricator', 'aphront/response/404'); phutil_require_module('phabricator', 'applications/phid/handle/data'); +phutil_require_module('phabricator', 'applications/phriction/constants/changetype'); phutil_require_module('phabricator', 'applications/phriction/controller/base'); phutil_require_module('phabricator', 'applications/phriction/storage/content'); phutil_require_module('phabricator', 'applications/phriction/storage/document'); diff --git a/src/applications/phriction/editor/document/PhrictionDocumentEditor.php b/src/applications/phriction/editor/document/PhrictionDocumentEditor.php index 55945d9477..9b7cd6de72 100644 --- a/src/applications/phriction/editor/document/PhrictionDocumentEditor.php +++ b/src/applications/phriction/editor/document/PhrictionDocumentEditor.php @@ -89,17 +89,53 @@ final class PhrictionDocumentEditor { return $this->document; } + public function delete() { + if (!$this->user) { + throw new Exception("Call setUser() before deleting a document!"); + } + + // TODO: Should we do anything about deleting an already-deleted document? + // We currently allow it. + + $document = $this->document; + $content = $this->content; + + $new_content = $this->buildContentTemplate($document, $content); + + $new_content->setChangeType(PhrictionChangeType::CHANGE_DELETE); + $new_content->setContent(''); + + return $this->updateDocument($document, $content, $new_content); + } + public function save() { if (!$this->user) { - throw new Exception("Call setUser() before save()!"); + throw new Exception("Call setUser() before updating a document!"); + } + + if ($this->newContent === '') { + // If this is an edit which deletes all the content, just treat it as + // a delete. NOTE: null means "don't change the content", not "delete + // the page"! Thus the strict type check. + return $this->delete(); } $document = $this->document; $content = $this->content; + $new_content = $this->buildContentTemplate($document, $content); + + return $this->updateDocument($document, $content, $new_content); + } + + private function buildContentTemplate( + PhrictionDocument $document, + PhrictionContent $content) { + $new_content = new PhrictionContent(); $new_content->setSlug($document->getSlug()); $new_content->setAuthorPHID($this->user->getPHID()); + $new_content->setChangeType(PhrictionChangeType::CHANGE_EDIT); $new_content->setTitle( coalesce( @@ -115,12 +151,44 @@ final class PhrictionDocumentEditor { $new_content->setDescription($this->description); } - $new_content->setVersion($content->getVersion() + 1); + return $new_content; + } + + private function updateDocument($document, $content, $new_content) { - // TODO: This should be transactional. $is_new = false; if (!$document->getID()) { $is_new = true; + } + + $new_content->setVersion($content->getVersion() + 1); + + $change_type = $new_content->getChangeType(); + switch ($change_type) { + case PhrictionChangeType::CHANGE_EDIT: + $doc_status = PhrictionDocumentStatus::STATUS_EXISTS; + $feed_action = $is_new + ? PhrictionActionConstants::ACTION_CREATE + : PhrictionActionConstants::ACTION_EDIT; + break; + case PhrictionChangeType::CHANGE_DELETE: + $doc_status = PhrictionDocumentStatus::STATUS_DELETED; + $feed_action = PhrictionActionConstants::ACTION_DELETE; + if ($is_new) { + throw new Exception( + "You can not delete a document which doesn't exist yet!"); + } + break; + default: + throw new Exception( + "Unsupported content change type '{$change_type}'!"); + } + + $document->setStatus($doc_status); + + // TODO: This should be transactional. + + if ($is_new) { $document->save(); } @@ -145,9 +213,7 @@ final class PhrictionDocumentEditor { ->setStoryData( array( 'phid' => $document->getPHID(), - 'action' => $is_new - ? PhrictionActionConstants::ACTION_CREATE - : PhrictionActionConstants::ACTION_EDIT, + 'action' => $feed_action, 'content' => phutil_utf8_shorten($new_content->getContent(), 140), )) ->publish(); diff --git a/src/applications/phriction/editor/document/__init__.php b/src/applications/phriction/editor/document/__init__.php index 1844b3b0c8..fcd94526d1 100644 --- a/src/applications/phriction/editor/document/__init__.php +++ b/src/applications/phriction/editor/document/__init__.php @@ -9,6 +9,8 @@ phutil_require_module('phabricator', 'applications/feed/constants/story'); phutil_require_module('phabricator', 'applications/feed/publisher'); phutil_require_module('phabricator', 'applications/phriction/constants/action'); +phutil_require_module('phabricator', 'applications/phriction/constants/changetype'); +phutil_require_module('phabricator', 'applications/phriction/constants/documentstatus'); phutil_require_module('phabricator', 'applications/phriction/storage/content'); phutil_require_module('phabricator', 'applications/phriction/storage/document'); phutil_require_module('phabricator', 'applications/search/index/indexer/phriction'); diff --git a/src/applications/phriction/storage/content/PhrictionContent.php b/src/applications/phriction/storage/content/PhrictionContent.php index ba486a4006..82c4145513 100644 --- a/src/applications/phriction/storage/content/PhrictionContent.php +++ b/src/applications/phriction/storage/content/PhrictionContent.php @@ -31,4 +31,7 @@ class PhrictionContent extends PhrictionDAO { protected $content; protected $description; + protected $changeType; + protected $changeRef; + } diff --git a/src/applications/phriction/storage/document/PhrictionDocument.php b/src/applications/phriction/storage/document/PhrictionDocument.php index 614f6b2dcd..9b966ec1d6 100644 --- a/src/applications/phriction/storage/document/PhrictionDocument.php +++ b/src/applications/phriction/storage/document/PhrictionDocument.php @@ -26,6 +26,7 @@ class PhrictionDocument extends PhrictionDAO { protected $slug; protected $depth; protected $contentID; + protected $status; private $contentObject;