From fd3ae9100f62d385f12be93e0bf32c668c842dd0 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 2 Mar 2011 15:38:31 +1300 Subject: [PATCH] API CHANGE Replaced LeftAndMain->ajaxupdatesort() and LeftAndMain->ajaxupdateparent() with a combined LeftAndMain->savetreenode() (and a changed request parameter signature) --- code/LeftAndMain.php | 140 +++++++++++++++++--------------------- tests/CMSMainTest.yml | 29 ++++++++ tests/LeftAndMainTest.php | 61 +++++++++++++++++ 3 files changed, 154 insertions(+), 76 deletions(-) diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index d30eb417..d690ae93 100644 --- a/code/LeftAndMain.php +++ b/code/LeftAndMain.php @@ -45,8 +45,7 @@ class LeftAndMain extends Controller { static $allowed_actions = array( 'index', - 'ajaxupdateparent', - 'ajaxupdatesort', + 'savetreenode', 'getitem', 'getsubtree', 'myprofile', @@ -614,9 +613,18 @@ class LeftAndMain extends Controller { } /** - * Ajax handler for updating the parent of a tree node + * Update the position and parent of a tree node. + * Only saves the node if changes were made. + * + * Required data: + * - 'ID': The moved node + * - 'ParentID': New parent relation of the moved node (0 for root) + * - 'SiblingIDs': Array of all sibling nodes to the moved node (incl. the node itself). + * In case of a 'ParentID' change, relates to the new siblings under the new parent. + * + * @return SS_HTTPResponse JSON string with a */ - public function ajaxupdateparent($request) { + public function savetreenode($request) { if (!Permission::check('SITETREE_REORGANISE') && !Permission::check('ADMIN')) { $this->response->setStatusCode( 403, @@ -625,88 +633,68 @@ class LeftAndMain extends Controller { return; } + $className = $this->stat('tree_class'); + $statusUpdates = array('modified'=>array()); $id = $request->requestVar('ID'); $parentID = $request->requestVar('ParentID'); + $siblingIDs = $request->requestVar('SiblingIDs'); $statusUpdates = array('modified'=>array()); - - if(is_numeric($id) && is_numeric($parentID) && $id != $parentID) { - $node = DataObject::get_by_id($this->stat('tree_class'), $id); - if($node){ - if($node && !$node->canEdit()) return Security::permissionFailure($this); - - $node->ParentID = $parentID; - $node->write(); - - $statusUpdates['modified'][$node->ID] = array( - 'TreeTitle'=>$node->TreeTitle - ); - - // Update all dependent pages - if($virtualPages = DataObject::get("VirtualPage", "\"CopyContentFromID\" = $node->ID")) { - foreach($virtualPages as $virtualPage) { - $statusUpdates['modified'][$virtualPage->ID] = array( - 'TreeTitle' => $virtualPage->TreeTitle() - ); - } - } - - $this->response->addHeader( - 'X-Status', - _t('LeftAndMain.SAVED','saved') - ); - }else{ - $this->response->setStatusCode( - 500, - _t( - 'LeftAndMain.PLEASESAVE', - "Please Save Page: This page could not be upated because it hasn't been saved yet." - ) - ); - } - } + if(!is_numeric($id) || !is_numeric($parentID)) throw new InvalidArgumentException(); - return Convert::raw2json($statusUpdates); - } - - /** - * Ajax handler for updating the order of a number of tree nodes - * $_GET[ID]: An array of node ids in the correct order - * $_GET[MovedNodeID]: The node that actually got moved - */ - public function ajaxupdatesort($request) { - if (!Permission::check('SITETREE_REORGANISE') && !Permission::check('ADMIN')) { + $node = DataObject::get_by_id($className, $id); + if($node && !$node->canEdit()) return Security::permissionFailure($this); + + if(!$node) { $this->response->setStatusCode( - 403, - _t('LeftAndMain.CANT_REORGANISE',"You do not have permission to rearange the site tree. Your change was not saved.") + 500, + _t( + 'LeftAndMain.PLEASESAVE', + "Please Save Page: This page could not be upated because it hasn't been saved yet." + ) ); return; } - $className = $this->stat('tree_class'); - $counter = 0; - $statusUpdates = array('modified'=>array()); - - if(!is_array($request->requestVar('ID'))) return false; - - //Sorting root - if($request->requestVar('MovedNodeID')==0){ - $movedNode = DataObject::get($className, "\"ParentID\"=0"); - }else{ - $movedNode = DataObject::get_by_id($className, $request->requestVar('MovedNodeID')); - } - foreach($request->requestVar('ID') as $id) { - if($id == $movedNode->ID) { - $movedNode->Sort = ++$counter; - $movedNode->write(); - $statusUpdates['modified'][$movedNode->ID] = array( - 'TreeTitle'=>$movedNode->TreeTitle - ); - } else if(is_numeric($id)) { - // Nodes that weren't "actually moved" shouldn't be registered as - // having been edited; do a direct SQL update instead - ++$counter; - DB::query("UPDATE \"$className\" SET \"Sort\" = $counter WHERE \"ID\" = '$id'"); + // Update hierarchy (only if ParentID changed) + if($node->ParentID != $parentID) { + $node->ParentID = (int)$parentID; + $node->write(); + + $statusUpdates['modified'][$node->ID] = array( + 'TreeTitle'=>$node->TreeTitle + ); + + // Update all dependent pages + if($virtualPages = DataObject::get("VirtualPage", "\"CopyContentFromID\" = $node->ID")) { + foreach($virtualPages as $virtualPage) { + $statusUpdates['modified'][$virtualPage->ID] = array( + 'TreeTitle' => $virtualPage->TreeTitle() + ); + } } + + $this->response->addHeader('X-Status', _t('LeftAndMain.SAVED','saved')); + } + + // Update sorting + if(is_array($siblingIDs)) { + $counter = 0; + foreach($siblingIDs as $id) { + if($id == $node->ID) { + $node->Sort = ++$counter; + $node->write(); + $statusUpdates['modified'][$node->ID] = array( + 'TreeTitle' => $node->TreeTitle + ); + } else if(is_numeric($id)) { + // Nodes that weren't "actually moved" shouldn't be registered as + // having been edited; do a direct SQL update instead + ++$counter; + DB::query(sprintf("UPDATE \"%s\" SET \"Sort\" = %d WHERE \"ID\" = '%d'", $className, $counter, $id)); + } + } + + $this->response->addHeader('X-Status', _t('LeftAndMain.SAVED','saved')); } return Convert::raw2json($statusUpdates); diff --git a/tests/CMSMainTest.yml b/tests/CMSMainTest.yml index 1c8944e8..f2883b39 100644 --- a/tests/CMSMainTest.yml +++ b/tests/CMSMainTest.yml @@ -1,65 +1,94 @@ Page: page1: Title: Page 1 + Sort: 1 page2: Title: Page 2 + Sort: 2 page3: Title: Page 3 + Sort: 3 page31: Title: Page 3.1 Parent: =>Page.page3 + Sort: 1 page32: Title: Page 3.2 Parent: =>Page.page3 + Sort: 2 page4: Title: Page 4 + Sort: 4 page5: Title: Page 5 + Sort: 5 page6: Title: Page 6 + Sort: 6 page7: Title: Page 7 + Sort: 7 page8: Title: Page 8 + Sort: 8 page9: Title: Page 9 + Sort: 9 page10: Title: Page 10 + Sort: 10 page11: Title: Page 11 + Sort: 11 page12: Title: Page 12 + Sort: 12 page13: Title: Page 13 + Sort: 13 page14: Title: Page 14 + Sort: 14 page15: Title: Page 15 + Sort: 15 page16: Title: Page 16 + Sort: 16 page17: Title: Page 17 + Sort: 17 page18: Title: Page 18 + Sort: 18 page19: Title: Page 19 + Sort: 19 page20: Title: Page 20 + Sort: 20 page21: Title: Page 21 + Sort: 21 page22: Title: Page 22 + Sort: 22 page23: Title: Page 23 + Sort: 23 page24: Title: Page 24 + Sort: 24 page25: Title: Page 25 + Sort: 25 page26: Title: Page 26 + Sort: 26 home: Title: Home URLSegment: home + Sort: 27 Group: admin: Title: Administrators diff --git a/tests/LeftAndMainTest.php b/tests/LeftAndMainTest.php index 96752eb7..8395cf0f 100644 --- a/tests/LeftAndMainTest.php +++ b/tests/LeftAndMainTest.php @@ -14,6 +14,67 @@ class LeftAndMainTest extends FunctionalTest { CMSMenu::populate_menu(); } + public function testSaveTreeNodeSorting() { + $this->loginWithPermission('ADMIN'); + + $rootPages = DataObject::get('SiteTree', '"ParentID" = 0'); // implicitly sorted + $siblingIDs = $rootPages->column('ID'); + $page1 = $rootPages->offsetGet(0); + $page2 = $rootPages->offsetGet(1); + $page3 = $rootPages->offsetGet(2); + + // Move page2 before page1 + $siblingIDs[0] = $page2->ID; + $siblingIDs[1] = $page1->ID; + $data = array( + 'SiblingIDs' => $siblingIDs, + 'ID' => $page2->ID, + 'ParentID' => 0 + ); + + $response = $this->post('admin/savetreenode', $data); + $this->assertEquals(200, $response->getStatusCode()); + $page1 = DataObject::get_by_id('SiteTree', $page1->ID, false); + $page2 = DataObject::get_by_id('SiteTree', $page2->ID, false); + $page3 = DataObject::get_by_id('SiteTree', $page3->ID, false); + + $this->assertEquals(2, $page1->Sort, 'Page1 is sorted after Page2'); + $this->assertEquals(1, $page2->Sort, 'Page2 is sorted before Page1'); + $this->assertEquals(3, $page3->Sort, 'Sort order for other pages is unaffected'); + } + + public function testSaveTreeNodeParentID() { + $this->loginWithPermission('ADMIN'); + + $page1 = $this->objFromFixture('Page', 'page1'); + $page2 = $this->objFromFixture('Page', 'page2'); + $page3 = $this->objFromFixture('Page', 'page3'); + $page31 = $this->objFromFixture('Page', 'page31'); + $page32 = $this->objFromFixture('Page', 'page32'); + + // Move page2 into page3, between page3.1 and page 3.2 + $siblingIDs = array( + $page31->ID, + $page2->ID, + $page32->ID + ); + $data = array( + 'SiblingIDs' => $siblingIDs, + 'ID' => $page2->ID, + 'ParentID' => $page3->ID + ); + $response = $this->post('admin/savetreenode', $data); + $this->assertEquals(200, $response->getStatusCode()); + $page2 = DataObject::get_by_id('SiteTree', $page2->ID, false); + $page31 = DataObject::get_by_id('SiteTree', $page31->ID, false); + $page32 = DataObject::get_by_id('SiteTree', $page32->ID, false); + + $this->assertEquals($page3->ID, $page2->ParentID, 'Moved page gets new parent'); + $this->assertEquals(1, $page31->Sort, 'Children pages before insertaion are unaffected'); + $this->assertEquals(2, $page2->Sort, 'Moved page is correctly sorted'); + $this->assertEquals(3, $page32->Sort, 'Children pages after insertion are resorted'); + } + /** * Test that CMS versions can be interpreted appropriately */