From d0c2558a1f126b19df05c7952442d6a372516a7a Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 13 Jun 2017 17:07:14 +1200 Subject: [PATCH 1/2] FIX Get page ID from document set and include it in $NewLink for add document buttons This helps the module to determine where to return a user to after they've uploaded a file, e.g. either back to a document set in a page context, back to a document set in the ModelAdmin context or back to the ModelAdmin for documents with no context. --- code/cms/DMSDocumentAddController.php | 25 +++---- code/cms/DMSGridFieldAddNewButton.php | 8 +++ tests/DMSGridFieldAddNewButtonTest.php | 47 ------------- tests/cms/DMSDocumentAddControllerTest.php | 4 +- tests/cms/DMSGridFieldAddNewButtonTest.php | 79 ++++++++++++++++++++++ 5 files changed, 99 insertions(+), 64 deletions(-) delete mode 100644 tests/DMSGridFieldAddNewButtonTest.php create mode 100644 tests/cms/DMSGridFieldAddNewButtonTest.php diff --git a/code/cms/DMSDocumentAddController.php b/code/cms/DMSDocumentAddController.php index 13bb0ab..42b0a95 100644 --- a/code/cms/DMSDocumentAddController.php +++ b/code/cms/DMSDocumentAddController.php @@ -35,26 +35,21 @@ class DMSDocumentAddController extends LeftAndMain public function currentPage() { $id = $this->currentPageID(); - - if ($id && is_numeric($id) && $id > 0) { - return Versioned::get_by_stage('SiteTree', 'Stage', sprintf( - 'ID = %s', - (int) $id - ))->first(); - } else { - // ID is either '0' or 'root' - return singleton('SiteTree'); + if ($id === 0) { + return SiteTree::singleton(); } + return parent::currentPage(); } /** - * Return fake-ID "root" if no ID is found (needed to upload files into the root-folder) + * Return fake-ID "root" if no ID is found (needed to upload files into the root-folder). Otherwise the page ID + * is passed in from the {@link DMSGridFieldAddNewButton}. * * @return int */ public function currentPageID() { - return ($result = parent::currentPageID()) === null ? 0 : $result; + return (int) $this->getRequest()->getVar('page_id'); } /** @@ -177,11 +172,11 @@ class DMSDocumentAddController extends LeftAndMain } /** - * Returns the link to be used to return the user after uploading a document. If a document set ID (dsid) is present - * then it will be redirected back to that document set in a page. + * Returns the link to be used to return the user after uploading a document. Scenarios: * - * If no document set ID is present then we assume that it has been added from the model admin, so redirect back to - * that instead. + * 1) Page context: page ID and document set ID provided, redirect back to the page and document set + * 2) Document set context: no page ID, document set ID provided, redirect back to document set in ModelAdmin + * 3) Document context: no page ID and no document set ID provided, redirect back to documents in ModelAdmin * * @return string */ diff --git a/code/cms/DMSGridFieldAddNewButton.php b/code/cms/DMSGridFieldAddNewButton.php index cfeb475..aafa9da 100644 --- a/code/cms/DMSGridFieldAddNewButton.php +++ b/code/cms/DMSGridFieldAddNewButton.php @@ -32,6 +32,14 @@ class DMSGridFieldAddNewButton extends GridFieldAddNewButton implements GridFiel $link = singleton('DMSDocumentAddController')->Link(); if ($this->getDocumentSetId()) { $link = Controller::join_links($link, '?dsid=' . $this->getDocumentSetId()); + + // Look for an associated page, but only share it if we're editing in a page context + $set = DMSDocumentSet::get()->byId($this->getDocumentSetId()); + if ($set && $set->exists() && $set->Page()->exists() + && Controller::curr() instanceof CMSPageEditController + ) { + $link = Controller::join_links($link, '?page_id=' . $set->Page()->ID); + } } $data = new ArrayData(array( diff --git a/tests/DMSGridFieldAddNewButtonTest.php b/tests/DMSGridFieldAddNewButtonTest.php deleted file mode 100644 index bced8a0..0000000 --- a/tests/DMSGridFieldAddNewButtonTest.php +++ /dev/null @@ -1,47 +0,0 @@ -gridField = GridField::create('TestGridField', false, $fakeList); - $this->button = new DMSGridFieldAddNewButton; - } - - /** - * Test that when no document set ID is present then it is not added to the URL for "add document" - */ - public function testNoPageIdInAddUrlWhenNotProvided() - { - $fragments = $this->button->getHTMLFragments($this->gridField); - $result = array_pop($fragments)->getValue(); - $this->assertNotContains('?dsid', $result); - } - - /** - * Test that when a document set ID is provided, it is added onto the "add document" link - */ - public function testPageIdAddedToLinkWhenProvided() - { - $this->button->setDocumentSetId(123); - - $fragments = $this->button->getHTMLFragments($this->gridField); - $result = array_pop($fragments)->getValue(); - $this->assertContains('?dsid=123', $result); - } -} diff --git a/tests/cms/DMSDocumentAddControllerTest.php b/tests/cms/DMSDocumentAddControllerTest.php index e07f85f..a88eb01 100644 --- a/tests/cms/DMSDocumentAddControllerTest.php +++ b/tests/cms/DMSDocumentAddControllerTest.php @@ -27,7 +27,7 @@ class DMSDocumentAddControllerTest extends FunctionalTest $this->assertInstanceOf('SiteTree', $this->controller->currentPage()); $this->assertEmpty($this->controller->currentPage()->ID); - $this->controller->setRequest(new SS_HTTPRequest('GET', '/', array('ID' => $page->ID))); + $this->controller->setRequest(new SS_HTTPRequest('GET', '/', array('page_id' => $page->ID))); $this->assertEquals($page->ID, $this->controller->currentPage()->ID, 'Specified page is loaded and returned'); } @@ -74,7 +74,7 @@ class DMSDocumentAddControllerTest extends FunctionalTest $this->assertContains('123', $this->controller->Backlink()); // Has page ID and document set ID - $request = new SS_HTTPRequest('GET', '/', array('dsid' => 123, 'ID' => 234)); + $request = new SS_HTTPRequest('GET', '/', array('dsid' => 123, 'page_id' => 234)); $this->controller->setRequest($request); $this->assertContains('admin/pages', $this->controller->Backlink()); $this->assertContains('123', $this->controller->Backlink()); diff --git a/tests/cms/DMSGridFieldAddNewButtonTest.php b/tests/cms/DMSGridFieldAddNewButtonTest.php new file mode 100644 index 0000000..32efa0d --- /dev/null +++ b/tests/cms/DMSGridFieldAddNewButtonTest.php @@ -0,0 +1,79 @@ +gridField = GridField::create('TestGridField', false, $fakeList); + $this->button = new DMSGridFieldAddNewButton; + } + + /** + * Test that when no document set ID is present then it is not added to the URL for "add document" + */ + public function testNoDocumentSetIdInAddUrlWhenNotProvided() + { + $this->assertNotContains('?dsid', $this->getButtonHtml()); + } + + /** + * Test that when a document set ID is provided, it is added onto the "add document" link + */ + public function testDocumentSetIdAddedToLinkWhenProvided() + { + $this->button->setDocumentSetId(123); + $this->assertContains('?dsid=123', $this->getButtonHtml()); + } + + /** + * If a set is saved and associated to a page, that page's ID should be added to the "add document" link to help + * to ensure the user gets redirected back to the correct place afterwards + */ + public function testPageIdIsAddedWhenAvailableViaDocumentSetRelationship() + { + $set = $this->objFromFixture('DMSDocumentSet', 'ds1'); + $this->button->setDocumentSetId($set->ID); + + $controller = new ContentController; + $controller->pushCurrent(); + + $result = $this->getButtonHtml(); + $this->assertContains('dsid=' . $set->ID, $result, 'Add new button contains document set ID'); + $this->assertNotContains('page_id=' . $set->Page()->ID, $result, 'No page ID when not editing in page context'); + + $controller = new CMSPageEditController; + $controller->pushCurrent(); + + $this->assertContains( + 'page_id=' . $set->Page()->ID, + $this->getButtonHtml(), + 'Button contains page ID when in edit page context' + ); + } + + /** + * Returns the HTML result of the "add new" button, used for DRY in test class + * + * @return string + */ + protected function getButtonHtml() + { + $fragments = $this->button->getHTMLFragments($this->gridField); + return array_pop($fragments)->getValue(); + } +} From 9d5e028d10d2dc9e034dbbc6d3c0d9c4a65968ec Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 14 Jun 2017 15:53:37 +1200 Subject: [PATCH 2/2] FIX Ensure that redirection back to pages works for SS 3.6 --- code/cms/DMSDocumentAddController.php | 20 ++++++++++++++++++-- tests/cms/DMSDocumentAddControllerTest.php | 9 +++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/code/cms/DMSDocumentAddController.php b/code/cms/DMSDocumentAddController.php index 42b0a95..5c62a45 100644 --- a/code/cms/DMSDocumentAddController.php +++ b/code/cms/DMSDocumentAddController.php @@ -197,10 +197,26 @@ class DMSDocumentAddController extends LeftAndMain return $modelAdmin->Link(); } + return $this->getPageEditLink($this->currentPageID(), $this->getRequest()->getVar('dsid')); + } + + /** + * Return a link to a page. In SS <= 3.5 this is loaded from the session, whereas in SS >= 3.6 this is set + * explicitly to a class property on CMSMain. This method checks whether the URL handler to detect this ID + * exists on CMSMain, if so include it in the URL. + * + * @param int $pageId + * @param int $documentSetId + * @return string + */ + protected function getPageEditLink($pageId, $documentSetId) + { + // Only get configuration from CMSMain, not its descendants or extensions + $urlHandlers = (array) Config::inst()->get('CMSMain', 'url_handlers', Config::UNINHERITED); + $pageIdSegment = array_key_exists('EditForm/$ID', $urlHandlers) ? $pageId . '/' : ''; return Controller::join_links( singleton('CMSPagesController')->Link(), - 'edit/EditForm/field/Document%20Sets/item', - $this->getRequest()->getVar('dsid') + sprintf('edit/EditForm/%sfield/Document Sets/item/%d', $pageIdSegment, $documentSetId) ); } diff --git a/tests/cms/DMSDocumentAddControllerTest.php b/tests/cms/DMSDocumentAddControllerTest.php index a88eb01..3fee715 100644 --- a/tests/cms/DMSDocumentAddControllerTest.php +++ b/tests/cms/DMSDocumentAddControllerTest.php @@ -78,6 +78,15 @@ class DMSDocumentAddControllerTest extends FunctionalTest $this->controller->setRequest($request); $this->assertContains('admin/pages', $this->controller->Backlink()); $this->assertContains('123', $this->controller->Backlink()); + + $urlHandlers = (array) Config::inst()->get('CMSMain', 'url_handlers', Config::UNINHERITED); + if (array_key_exists('EditForm/$ID', $urlHandlers)) { + // SS 3.6 and above, ensure that the page ID is in the edit URL + $this->assertContains('admin/pages/edit/EditForm/234', $this->controller->Backlink()); + } else { + // SS 3.5 and below, the page ID is loaded from the session + $this->assertNotContains('234', $this->controller->Backlink()); + } } /**