From a965d8d6ae5ac54744b0bbc07107b53e84493e51 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Feb 2018 05:42:01 -0800 Subject: [PATCH] Make PhrictionContent "description" non-nullable Summary: Depends on D19095. Ref T6203. Ref T13077. This column is nullable in an inconsistent way. Make it non-nullable. Also clean up one more content query on the history view. Test Plan: Ran migration, then created and edited documents without providing a descriptino or hitting `NULL` exceptions. Maniphest Tasks: T13077, T6203 Differential Revision: https://secure.phabricator.com/D19096 --- .../20180215.phriction.03.descempty.sql | 2 ++ .../20180215.phriction.04.descnull.sql | 2 ++ .../conduit/PhrictionCreateConduitAPIMethod.php | 2 +- .../conduit/PhrictionEditConduitAPIMethod.php | 2 +- .../controller/PhrictionHistoryController.php | 15 ++++++--------- .../editor/PhrictionTransactionEditor.php | 5 ++++- .../phriction/storage/PhrictionContent.php | 5 +---- 7 files changed, 17 insertions(+), 16 deletions(-) create mode 100644 resources/sql/autopatches/20180215.phriction.03.descempty.sql create mode 100644 resources/sql/autopatches/20180215.phriction.04.descnull.sql diff --git a/resources/sql/autopatches/20180215.phriction.03.descempty.sql b/resources/sql/autopatches/20180215.phriction.03.descempty.sql new file mode 100644 index 0000000000..c41df5285a --- /dev/null +++ b/resources/sql/autopatches/20180215.phriction.03.descempty.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_phriction.phriction_content + SET description = '' WHERE description IS NULL; diff --git a/resources/sql/autopatches/20180215.phriction.04.descnull.sql b/resources/sql/autopatches/20180215.phriction.04.descnull.sql new file mode 100644 index 0000000000..3ff017cd64 --- /dev/null +++ b/resources/sql/autopatches/20180215.phriction.04.descnull.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_phriction.phriction_content + CHANGE description description LONGTEXT NOT NULL COLLATE {$COLLATE_TEXT}; diff --git a/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php index fc28b53b66..300dbbfa80 100644 --- a/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php +++ b/src/applications/phriction/conduit/PhrictionCreateConduitAPIMethod.php @@ -57,7 +57,7 @@ final class PhrictionCreateConduitAPIMethod extends PhrictionConduitAPIMethod { ->setActor($request->getUser()) ->setContentSource($request->newContentSource()) ->setContinueOnNoEffect(true) - ->setDescription($request->getValue('description')); + ->setDescription((string)$request->getValue('description')); try { $editor->applyTransactions($doc, $xactions); diff --git a/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php b/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php index 70c02d376a..9d97ed7f8c 100644 --- a/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php +++ b/src/applications/phriction/conduit/PhrictionEditConduitAPIMethod.php @@ -52,7 +52,7 @@ final class PhrictionEditConduitAPIMethod extends PhrictionConduitAPIMethod { ->setActor($request->getUser()) ->setContentSource($request->newContentSource()) ->setContinueOnNoEffect(true) - ->setDescription($request->getValue('description')); + ->setDescription((string)$request->getValue('description')); try { $editor->applyTransactions($doc, $xactions); diff --git a/src/applications/phriction/controller/PhrictionHistoryController.php b/src/applications/phriction/controller/PhrictionHistoryController.php index 9b0d3188a2..432b1dfdef 100644 --- a/src/applications/phriction/controller/PhrictionHistoryController.php +++ b/src/applications/phriction/controller/PhrictionHistoryController.php @@ -22,16 +22,13 @@ final class PhrictionHistoryController $current = $document->getContent(); - $pager = new PHUIPagerView(); - $pager->setOffset($request->getInt('page')); - $pager->setURI($request->getRequestURI(), 'page'); + $pager = id(new AphrontCursorPagerView()) + ->readFromRequest($request); - $history = id(new PhrictionContent())->loadAllWhere( - 'documentID = %d ORDER BY version DESC LIMIT %d, %d', - $document->getID(), - $pager->getOffset(), - $pager->getPageSize() + 1); - $history = $pager->sliceResults($history); + $history = id(new PhrictionContentQuery()) + ->setViewer($viewer) + ->withDocumentPHIDs(array($document->getPHID())) + ->executeWithCursorPager($pager); $author_phids = mpull($history, 'getAuthorPHID'); $handles = $this->loadViewerHandles($author_phids); diff --git a/src/applications/phriction/editor/PhrictionTransactionEditor.php b/src/applications/phriction/editor/PhrictionTransactionEditor.php index 73aee3fd4c..793c609517 100644 --- a/src/applications/phriction/editor/PhrictionTransactionEditor.php +++ b/src/applications/phriction/editor/PhrictionTransactionEditor.php @@ -596,10 +596,13 @@ final class PhrictionTransactionEditor ->setAuthorPHID($this->getActor()->getPHID()) ->setChangeType(PhrictionChangeType::CHANGE_EDIT) ->setTitle($this->getOldContent()->getTitle()) - ->setContent($this->getOldContent()->getContent()); + ->setContent($this->getOldContent()->getContent()) + ->setDescription(''); + if (strlen($this->getDescription())) { $new_content->setDescription($this->getDescription()); } + $new_content->setVersion($this->getOldContent()->getVersion() + 1); return $new_content; diff --git a/src/applications/phriction/storage/PhrictionContent.php b/src/applications/phriction/storage/PhrictionContent.php index a6aee7226f..390da64682 100644 --- a/src/applications/phriction/storage/PhrictionContent.php +++ b/src/applications/phriction/storage/PhrictionContent.php @@ -30,10 +30,7 @@ final class PhrictionContent 'content' => 'text', 'changeType' => 'uint32', 'changeRef' => 'uint32?', - - // T6203/NULLABILITY - // This should just be empty if not provided? - 'description' => 'text?', + 'description' => 'text', ), self::CONFIG_KEY_SCHEMA => array( 'documentID' => array(