So... see if you can spot what's wrong with this code :)
$_batch = $batch;
$batch = NULL;
// Clean-up the session. Not needed for CLI updates.
if (isset($_SESSION)) {
unset($_SESSION['batches'][$batch['id']]);
if (empty($_SESSION['batches'])) {
unset($_SESSION['batches']);
}
}
Attached patch fixes it.
This was fixed in #1215104: Use the non-interactive installer in WebTestBase::setUp() but tests need to be added.
Add a test to show that batch ids are removed from the session.
Review
Commit
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | 1130122-20.patch | 1.06 KB | quietone |
| #7 | drupal-1130122-batchapi-cleanup-tests-and-fix-7.patch | 1.91 KB | mgifford |
| #4 | drupal-1130122-batchapi-cleanup-tests-and-fix.patch | 1.89 KB | steven jones |
| #3 | drupal-1130122-batchapi-cleanup-tests.patch | 1.44 KB | steven jones |
| batch-id-session-unset.patch | 447 bytes | David_Rothstein |
Comments
Comment #1
aspilicious commentedSounds logical...
In case you didn't noticed:
-------------------------
$batch will be copied into $_batch.
Afterwards we make $batch empty.
Thats why the ID is stored in $_batch.
ps: does this needs a test?
Comment #2
aspilicious commentedBack to needs review for the test question
Comment #3
steven jones commentedGuess we should probably have a test that ensures that the session is cleaned up after the batch is run.
Comment #4
steven jones commentedAnd hopefully a fix for the issue as per the original post.
Comment #7
mgiffordreroll.
Comment #9
David_Rothstein commentedIt looks like this was fixed inadvertently in Drupal 8 by #1215104: Use the non-interactive installer in WebTestBase::setUp(), but the tests here would still be good to get into Drupal 8.
Bug itself still exists in Drupal 7 also.
Comment #20
quietone commentedInstead of making a new functional tests I took the risk of adding a couple of lines to an existing Batch test, system/tests/src/Functional/Batch/ProcessingTest.php. That does requires adding the 'session_test' modules to all the tests in that file. I am hoping that is OK.
Adding a test is Task.