Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
migration system
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
11 Aug 2015 at 21:43 UTC
Updated:
25 Sep 2015 at 21:34 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
lostkangaroo commentedHelps to upload the patch
Comment #3
lostkangaroo commentedComment #4
mikeryanCould you add a test demonstrating the failure?
Exactly what scenario triggered it? Would it be better to fix the thrower that didn't properly form the array?
Comment #5
lostkangaroo commentedTo be quick, this was encountered when a module was disabled in D6 when a migration package included it as a source dependency. There was only one requirement, in this case "source provider", with only one value. Following the trail this value gets sent as an array to RequirementsException as a flat array as opposed to the 2D array that is needed for the nested foreach loops of getRequirementsString(). I would say that it is a good idea to fix the thrower of the exception to avoid this but we would need to guarantee that
$this->pluginDefinition['source_provider']always returns an array to do so.Tests are always a good idea and will look to add some shortly.
Comment #6
lostkangaroo commentedRunning the tests solo without the fix patch to show the error.
Comment #7
lostkangaroo commentedComment #8
lostkangaroo commentedTests with fix applied.
Comment #10
phenaproximaLooks good except for two things:
Code repetition makes me scrunch up my face. Maybe do something like this:
Should use camelCase.
Comment #11
phenaproximaNit: missing a period.
Comment #12
phenaproximaOne more.
The core convention is to use the word "provider" in data provider names. So maybe getRequirementsProvider() is a better name for this method.
Comment #13
lostkangaroo commentedAddressed all from #10, #11, and #12
I chose to use
!is_array()rather thenis_scalarbecause according to the second note on http://php.net/manual/en/function.is-scalar.php NULL values will be passed through which would cause the original php error.Comment #14
phenaproximaTiny annoying code style nit: that should be
if (!is_array($requirements)). Can be fixed on commit, though.This has passed DrupalCI and otherwise looks fine to me. RTBC!
Comment #15
webchickFixed that, and...
Committed and pushed to 8.0.x. Thanks!
Although those random test names are truly... random. :)