Commit afc2f852 authored by epriestley's avatar epriestley
Browse files

Allow Phriction documents to be deleted

Summary:
  - Add a "delete" operation. Delete is just a special edit which removes the
page from indexes and shows a notice that the document has been deleted.
  - When a user deletes all the content on a page, treat it as a delete.
  - When a conduit call deletes all the content on a page, treat it as a delete.
  - Add page status to Conduit.
  - Add change type field to history.
  - Added a couple of constants to support a future 'move' change, which would
move content from one document to another.

Test Plan:
  - Verified deleted pages vanish from the document index (and restoring them
puts them back).
  - Verified deleted pages show "This page has been deleted...".
  - Created, edited and deleted a document via Conduit.
  - Deleted pages via "delete" button.
  - Deleted pages via editing content to nothing.

Reviewers: btrahan, jungejason

Reviewed By: btrahan

CC: skrul, aran, btrahan, epriestley

Maniphest Tasks: T680

Differential Revision: 1230
parent 9d372dd5
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
......@@ -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',
......
......@@ -365,6 +365,7 @@ class AphrontDefaultApplicationConfiguration
'history/(?P<slug>.+/)$' => 'PhrictionHistoryController',
'edit/(?:(?P<id>\d+)/)?$' => 'PhrictionEditController',
'delete/(?P<id>\d+)/$' => 'PhrictionDeleteController',
'preview/$' => 'PhrictionDocumentPreviewController',
'diff/(?P<id>\d+)/$' => 'PhrictionDiffController',
......
......@@ -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(),
);
......
......@@ -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');
......
......@@ -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");
......
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @group phriction
*/
final class PhrictionChangeType extends PhrictionConstants {
const CHANGE_EDIT = 0;
const CHANGE_DELETE = 1;
const CHANGE_MOVE_HERE = 2;
const CHANGE_MOVE_AWAY = 3;
public static function getChangeTypeLabel($const) {
static $map = array(
self::CHANGE_EDIT => 'Edit',
self::CHANGE_DELETE => 'Delete',
self::CHANGE_MOVE_HERE => 'Move Here',
self::CHANGE_MOVE_AWAY => 'Move Away',
);
return idx($map, $const, '???');
}
}
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/phriction/constants/base');
phutil_require_module('phutil', 'utils');
phutil_require_source('PhrictionChangeType.php');
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @group phriction
*/
final class PhrictionDocumentStatus extends PhrictionConstants {
const STATUS_EXISTS = 0;
const STATUS_DELETED = 1;
public static function getConduitConstant($const) {
static $map = array(
self::STATUS_EXISTS => 'exists',
self::STATUS_DELETED => 'deleted',
);
return idx($map, $const, 'unknown');
}
}
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'applications/phriction/constants/base');
phutil_require_module('phutil', 'utils');
phutil_require_source('PhrictionDocumentStatus.php');
<?php
/*
* Copyright 2011 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/**
* @group phriction
*/
class PhrictionDeleteController extends PhrictionController {
private $id;
public function willProcessRequest(array $data) {
$this->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);
}
}
<?php
/**
* This file is automatically generated. Lint this module to rebuild it.
* @generated
*/
phutil_require_module('phabricator', 'aphront/response/404');
phutil_require_module('phabricator', 'aphront/response/dialog');
phutil_require_module('phabricator', 'aphront/response/redirect');
phutil_require_module('phabricator', 'applications/phriction/controller/base');
phutil_require_module('phabricator', 'applications/phriction/editor/document');
phutil_require_module('phabricator', 'applications/phriction/storage/document');
phutil_require_module('phabricator', 'view/dialog');
phutil_require_module('phutil', 'utils');
phutil_require_source('PhrictionDeleteController.php');
......@@ -205,6 +205,12 @@ class PhrictionDiffController
$document_id = $content->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',
......
......@@ -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');
......
......@@ -120,12 +120,28 @@ class PhrictionDocumentController
$engine = PhabricatorMarkupEngine::newPhrictionMarkupEngine();
$doc_status = $document->getStatus();
if ($doc_status == PhrictionDocumentStatus::STATUS_EXISTS) {
$core_content =
'<div class="phabricator-remarkup">'.
$engine->markupText($content->getContent()).
'</div>';
} 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 =
'<div class="phriction-content">'.
$byline.
'<div class="phabricator-remarkup">'.
$engine->markupText($content->getContent()).
'</div>'.
$core_content.
'</div>';
$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 = '<em>'.$item.'</em>';
......
......@@ -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');
......
......@@ -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 =
'<div class="aphront-panel-preview aphront-panel-preview-wide">
<div class="phriction-document-preview-header">
......
......@@ -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',
'',
'',
......
......@@ -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');
......
......@@ -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(</