From e59a45f1562b6815dfbc84357973f1cf5a2833fd Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Thu, 6 Aug 2015 13:19:12 +0100 Subject: [PATCH] Issue #2426969 by David_Rothstein, joelpittet, dawehner: Dynamic redirects are no longer possible in the batch API (breaks updating existing modules or themes with the Update Manager) --- core/includes/batch.inc | 18 +++++++++++++++--- core/includes/form.inc | 9 ++++++++- .../system/src/Tests/Batch/ProcessingTest.php | 15 ++++++++++++++- .../batch_test/batch_test.callbacks.inc | 12 ++++++++++++ .../modules/batch_test/batch_test.routing.yml | 8 ++++++++ .../src/Controller/BatchTestController.php | 15 +++++++++++++++ 6 files changed, 72 insertions(+), 5 deletions(-) diff --git a/core/includes/batch.inc b/core/includes/batch.inc index 7aa8ee794226..cc705572b813 100644 --- a/core/includes/batch.inc +++ b/core/includes/batch.inc @@ -406,6 +406,7 @@ function _batch_next_set() { */ function _batch_finished() { $batch = &batch_get(); + $batch_finished_redirect = NULL; // Execute the 'finished' callbacks for each batch set, if defined. foreach ($batch['sets'] as $batch_set) { @@ -417,7 +418,13 @@ function _batch_finished() { if (is_callable($batch_set['finished'])) { $queue = _batch_queue($batch_set); $operations = $queue->getAllItems(); - call_user_func_array($batch_set['finished'], array($batch_set['success'], $batch_set['results'], $operations, \Drupal::service('date.formatter')->formatInterval($batch_set['elapsed'] / 1000))); + $batch_set_result = call_user_func_array($batch_set['finished'], array($batch_set['success'], $batch_set['results'], $operations, \Drupal::service('date.formatter')->formatInterval($batch_set['elapsed'] / 1000))); + // If a batch 'finished' callback requested a redirect after the batch + // is complete, save that for later use. If more than one batch set + // returned a redirect, the last one is used. + if ($batch_set_result instanceof RedirectResponse) { + $batch_finished_redirect = $batch_set_result; + } } } } @@ -448,8 +455,13 @@ function _batch_finished() { \Drupal::request()->query->set('destination', $_batch['destination']); } - // Determine the target path to redirect to. - if (!isset($_batch['form_state'])) { + // Determine the target path to redirect to. If a batch 'finished' callback + // returned a redirect response object, use that. Otherwise, fall back on + // the form redirection. + if (isset($batch_finished_redirect)) { + return $batch_finished_redirect; + } + elseif (!isset($_batch['form_state'])) { $_batch['form_state'] = new FormState(); } if ($_batch['form_state']->getRedirect() === NULL) { diff --git a/core/includes/form.inc b/core/includes/form.inc index 1c88201762ac..7dfcce2ce325 100644 --- a/core/includes/form.inc +++ b/core/includes/form.inc @@ -770,7 +770,14 @@ function batch_set($batch_definition) { * * @param \Drupal\Core\Url|string $redirect * (optional) Either path or Url object to redirect to when the batch has - * finished processing. + * finished processing. Note that to simply force a batch to (conditionally) + * redirect to a custom location after it is finished processing but to + * otherwise allow the standard form API batch handling to occur, it is not + * necessary to call batch_process() and use this parameter. Instead, make + * the batch 'finished' callback return an instance of + * \Symfony\Component\HttpFoundation\RedirectResponse, which will be used + * automatically by the standard batch processing pipeline (and which takes + * precedence over this parameter). * @param \Drupal\Core\Url $url * (optional - should only be used for separate scripts like update.php) * URL of the batch processing page. diff --git a/core/modules/system/src/Tests/Batch/ProcessingTest.php b/core/modules/system/src/Tests/Batch/ProcessingTest.php index fe198230ed5b..4d3412cf071c 100644 --- a/core/modules/system/src/Tests/Batch/ProcessingTest.php +++ b/core/modules/system/src/Tests/Batch/ProcessingTest.php @@ -7,6 +7,7 @@ namespace Drupal\system\Tests\Batch; +use Drupal\Core\Url; use Drupal\simpletest\WebTestBase; /** @@ -21,7 +22,7 @@ class ProcessingTest extends WebTestBase { * * @var array */ - public static $modules = array('batch_test'); + public static $modules = array('batch_test', 'test_page_test'); /** * Tests batches triggered outside of form submission. @@ -34,6 +35,18 @@ function testBatchNoForm() { $this->assertText('Redirection successful.', 'Redirection after batch execution is correct.'); } + /** + * Tests batches that redirect in the batch finished callback. + */ + function testBatchRedirectFinishedCallback() { + // Displaying the page triggers batch 1. + $this->drupalGet('batch-test/finish-redirect'); + $this->assertBatchMessages($this->_resultMessages('batch_1'), 'Batch for step 2 performed successfully.'); + $this->assertEqual(batch_test_stack(), $this->_resultStack('batch_1'), 'Execution order was correct.'); + $this->assertText('Test page text.', 'Custom redirection after batch execution displays the correct page.'); + $this->assertUrl(Url::fromRoute('test_page_test.test_page')); + } + /** * Tests batches defined in a form submit handler. */ diff --git a/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc b/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc index 6fc9a8d083c1..b1f27960e9f2 100644 --- a/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc +++ b/core/modules/system/tests/modules/batch_test/batch_test.callbacks.inc @@ -6,6 +6,8 @@ */ use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Url; +use Symfony\Component\HttpFoundation\RedirectResponse; /** * Implements callback_batch_operation(). @@ -123,6 +125,16 @@ function _batch_test_finished_1($success, $results, $operations) { _batch_test_finished_helper(1, $success, $results, $operations); } +/** + * Implements callback_batch_finished(). + * + * Triggers 'finished' callback for batch 1. + */ +function _batch_test_finished_1_finished($success, $results, $operations) { + _batch_test_finished_helper(1, $success, $results, $operations); + return new RedirectResponse(Url::fromRoute('test_page_test.test_page', [], ['absolute' => TRUE])->toString()); +} + /** * Implements callback_batch_finished(). * diff --git a/core/modules/system/tests/modules/batch_test/batch_test.routing.yml b/core/modules/system/tests/modules/batch_test/batch_test.routing.yml index 638d6fe9494d..5b5a117af0ac 100644 --- a/core/modules/system/tests/modules/batch_test/batch_test.routing.yml +++ b/core/modules/system/tests/modules/batch_test/batch_test.routing.yml @@ -31,6 +31,14 @@ batch_test.no_form: requirements: _access: 'TRUE' +batch_test.finish_redirect: + path: '/batch-test/finish-redirect' + defaults: + _controller: '\Drupal\batch_test\Controller\BatchTestController::testFinishRedirect' + _title: 'Simple page with finish redirect call' + requirements: + _access: 'TRUE' + batch_test.test_form: path: '/batch-test' defaults: diff --git a/core/modules/system/tests/modules/batch_test/src/Controller/BatchTestController.php b/core/modules/system/tests/modules/batch_test/src/Controller/BatchTestController.php index 19ea6e9bf346..35e5bea6ddf9 100644 --- a/core/modules/system/tests/modules/batch_test/src/Controller/BatchTestController.php +++ b/core/modules/system/tests/modules/batch_test/src/Controller/BatchTestController.php @@ -72,6 +72,21 @@ public function testNoForm() { } + /** + * Fires a batch process without a form submission and a finish redirect. + * + * @return \Symfony\Component\HttpFoundation\RedirectResponse|null + * A redirect response if the batch is progressive. No return value otherwise. + */ + public function testFinishRedirect() { + batch_test_stack(NULL, TRUE); + + $batch = _batch_test_batch_1(); + $batch['finished'] = '_batch_test_finished_1_finished'; + batch_set($batch); + return batch_process('batch-test/redirect'); + } + /** * Submits the 'Chained' form programmatically. * -- GitLab