From fd016b586485b518cccb52511bfc9948c12f190f Mon Sep 17 00:00:00 2001 From: Anh Nhan Nguyen Date: Mon, 4 Mar 2013 07:31:59 -0800 Subject: [PATCH] Creating (or updating) a Phriction Document creates empty parent documents Summary: Fixes T2208 Parent documents get created as stubs - while the action is "parent" - don't ask Test Plan: Created some documents with somewhat ancestry - verified their parents had pages and stub messages display ({F34456}). Reviewers: epriestley, btrahan Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2208 Differential Revision: https://secure.phabricator.com/D5201 --- .../constants/PhrictionChangeType.php | 2 + .../constants/PhrictionDocumentStatus.php | 2 + .../PhrictionDocumentController.php | 15 ++++- .../editor/PhrictionDocumentEditor.php | 65 +++++++++++++++---- 4 files changed, 69 insertions(+), 15 deletions(-) diff --git a/src/applications/phriction/constants/PhrictionChangeType.php b/src/applications/phriction/constants/PhrictionChangeType.php index 867e805f7d..ff50d58910 100644 --- a/src/applications/phriction/constants/PhrictionChangeType.php +++ b/src/applications/phriction/constants/PhrictionChangeType.php @@ -9,6 +9,7 @@ final class PhrictionChangeType extends PhrictionConstants { const CHANGE_DELETE = 1; const CHANGE_MOVE_HERE = 2; const CHANGE_MOVE_AWAY = 3; + const CHANGE_PARENT = 4; public static function getChangeTypeLabel($const) { static $map = array( @@ -16,6 +17,7 @@ final class PhrictionChangeType extends PhrictionConstants { self::CHANGE_DELETE => 'Delete', self::CHANGE_MOVE_HERE => 'Move Here', self::CHANGE_MOVE_AWAY => 'Move Away', + self::CHANGE_PARENT => 'Created through child', ); return idx($map, $const, '???'); diff --git a/src/applications/phriction/constants/PhrictionDocumentStatus.php b/src/applications/phriction/constants/PhrictionDocumentStatus.php index de75ee9107..dc33124c1d 100644 --- a/src/applications/phriction/constants/PhrictionDocumentStatus.php +++ b/src/applications/phriction/constants/PhrictionDocumentStatus.php @@ -7,11 +7,13 @@ final class PhrictionDocumentStatus extends PhrictionConstants { const STATUS_EXISTS = 0; const STATUS_DELETED = 1; + const STATUS_STUB = 3; public static function getConduitConstant($const) { static $map = array( self::STATUS_EXISTS => 'exists', self::STATUS_DELETED => 'deleted', + self::STATUS_STUB => 'stubbed', ); return idx($map, $const, 'unknown'); diff --git a/src/applications/phriction/controller/PhrictionDocumentController.php b/src/applications/phriction/controller/PhrictionDocumentController.php index fd7537dda5..ec0666d268 100644 --- a/src/applications/phriction/controller/PhrictionDocumentController.php +++ b/src/applications/phriction/controller/PhrictionDocumentController.php @@ -140,6 +140,14 @@ final class PhrictionDocumentController pht('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 if ($doc_status == PhrictionDocumentStatus::STATUS_STUB) { + $notice = new AphrontErrorView(); + $notice->setSeverity(AphrontErrorView::SEVERITY_NOTICE); + $notice->setTitle('Empty Document'); + $notice->appendChild( + pht('This document is empty. You can edit it to put some proper '. + 'content here.')); + $core_content = $notice->render(); } else { throw new Exception("Unknown document status '{$doc_status}'!"); } @@ -244,14 +252,17 @@ final 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 + AND d.status IN (%Ld) ORDER BY d.depth, c.title LIMIT %d', $document_dao->getTableName(), $content_dao->getTableName(), ($slug == '/' ? '' : $slug), $d_child, $d_grandchild, - PhrictionDocumentStatus::STATUS_EXISTS, + array( + PhrictionDocumentStatus::STATUS_EXISTS, + PhrictionDocumentStatus::STATUS_STUB, + ), $limit); if (!$children) { diff --git a/src/applications/phriction/editor/PhrictionDocumentEditor.php b/src/applications/phriction/editor/PhrictionDocumentEditor.php index fb35dbe73a..0f27ef5e2b 100644 --- a/src/applications/phriction/editor/PhrictionDocumentEditor.php +++ b/src/applications/phriction/editor/PhrictionDocumentEditor.php @@ -83,6 +83,18 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { return $this->updateDocument($document, $content, $new_content); } + public function stub() { + $actor = $this->requireActor(); + $document = $this->document; + $content = $this->content; + $new_content = $this->buildContentTemplate($document, $content); + + $new_content->setChangeType(PhrictionChangeType::CHANGE_PARENT); + $new_content->setContent(''); + + return $this->updateDocument($document, $content, $new_content); + } + public function save() { $actor = $this->requireActor(); @@ -152,6 +164,10 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { "You can not delete a document which doesn't exist yet!"); } break; + case PhrictionChangeType::CHANGE_PARENT: + $doc_status = PhrictionDocumentStatus::STATUS_STUB; + $feed_action = null; + break; default: throw new Exception( "Unsupported content change type '{$change_type}'!"); @@ -176,6 +192,27 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { id(new PhabricatorSearchIndexer()) ->indexDocumentByPHID($document->getPHID()); + // Stub out empty parent documents if they don't exist + $ancestral_slugs = PhabricatorSlug::getAncestry($document->getSlug()); + if ($ancestral_slugs) { + $ancestors = id(new PhrictionDocument())->loadAllWhere( + 'slug IN (%Ls)', + $ancestral_slugs); + foreach ($ancestral_slugs as $slug) { + // We check for change type to prevent near-infinite recursion + if (!isset($ancestors[$slug]) && + $new_content->getChangeType() != PhrictionChangeType::CHANGE_PARENT) { + + id(PhrictionDocumentEditor::newForSlug($slug)) + ->setActor($this->getActor()) + ->setTitle(PhabricatorSlug::getDefaultTitle($slug)) + ->setContent('') + ->setDescription(pht('Empty Parent Document')) + ->stub(); + } + } + } + $project_phid = null; $slug = $document->getSlug(); if (PhrictionDocument::isProjectSlug($slug)) { @@ -196,19 +233,21 @@ final class PhrictionDocumentEditor extends PhabricatorEditor { $related_phids[] = $project_phid; } - id(new PhabricatorFeedStoryPublisher()) - ->setRelatedPHIDs($related_phids) - ->setStoryAuthorPHID($this->getActor()->getPHID()) - ->setStoryTime(time()) - ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_PHRICTION) - ->setStoryData( - array( - 'phid' => $document->getPHID(), - 'action' => $feed_action, - 'content' => phutil_utf8_shorten($new_content->getContent(), 140), - 'project' => $project_phid, - )) - ->publish(); + if ($feed_action) { + id(new PhabricatorFeedStoryPublisher()) + ->setRelatedPHIDs($related_phids) + ->setStoryAuthorPHID($this->getActor()->getPHID()) + ->setStoryTime(time()) + ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_PHRICTION) + ->setStoryData( + array( + 'phid' => $document->getPHID(), + 'action' => $feed_action, + 'content' => phutil_utf8_shorten($new_content->getContent(), 140), + 'project' => $project_phid, + )) + ->publish(); + } return $this; }