Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
When adding a callback in an included file, can error if the function hasn't already been included.
TypeError: Argument 1 passed to Drupal\Core\Batch\BatchBuilder::addOperation() must be callable, string given...
To fix Drupal\Core\Batch\BatchBuilder::setFile()
needs to include the file that contains the callback function.
Comment | File | Size | Author |
---|---|---|---|
#18 | interdiff-17-18.txt | 760 bytes | jungle |
#18 | 3028621-18.patch | 2.1 KB | jungle |
Comments
Comment #2
John Cook CreditAttribution: John Cook at Creode commented@joachim brought this up while reviewing #2875279: Update core modules to use the new batch builder (https://www.drupal.org/project/drupal/issues/2875279#comment-12805887).
Comment #3
John Cook CreditAttribution: John Cook at Creode commentedI've made a failing test to show the problem.
Comment #5
John Cook CreditAttribution: John Cook at Creode commentedI've created a patch that includes the requested file. The
testSetFile()
test has been updated to reference an actual file as an error occurs if PHP is requested to include a file that does not exist.Comment #6
John Cook CreditAttribution: John Cook at Creode commentedComment #7
John Cook CreditAttribution: John Cook at Creode commentedMoved
batch_test_callback_in_file.php
to reflect @alexpott's comment in #2875279: Update core modules to use the new batch builder #24.Comment #11
jungleThanks @John Cook!
An existed callbacks file could be reused, instead of adding a new one.
This test does not cover that callbacks in the file are callables.
The test here is unnecessary to me. which could be a part of
::testSetFile()
Comment #13
pratik_kambleComment #14
jungleComment #15
pratik_kamble@jungle, I have reviewed the patch in comment #11 for file 3028621-11.patch.
Using the existing
batch_test.callbacks.inc
make sense. Verified the code intestSetFile()
function it LGTM.Marking it as RTBC.
Comment #16
kim.pepperNit:
You should use dirname(__DIR__, 6) here.
Comment #17
jungleThanks, @pratik_kamble and @kim.pepper for reviewing!
Addressing #16.
Comment #18
jungleSelf review:
One redundant whitespace, fixing.
Comment #19
jungleBTW, filed an issue for similar nitpick with #16
Comment #22
larowlanLooked into this for my curiosity.
This was only causing issues from the callable type-hint on the
BatchBuilder::addOperation
and::setFinishCallback
.The added
include_once
is in order for those type-hints to resolve.The actual loading of the file for the sake of processing the batch happens in
_batch_process
and_batch_finished
Committed ad50981 and pushed to 9.1.x. Thanks!
Cherry-picked to 9.0.x
Queued an 8.9.x test-run and flagging this for [backport]
Comment #24
larowlan#18 passes on 8.9.x.
As this is a bug and there is little disruption, cherry-picked to 8.9.x