I am generating unique ids from some other fields being imported (via csv/spreadsheet upload), which means this unique field isn't meant to be provided by the user directly. So it has a blank label in it's source. However, when users download the template file, they get a blank column (no header) as the first column. This is because the FeedsCSVParser->getTemplate() merges unique fields and sources together, with the uniques as the first argument of array_merge()
foreach (array_merge($uniques, $sources) as $col) {
In other words, the blank column is first regardless of the order it is defined in the processor. Understandably, this may be a bit of a specific use case, and doesn't really cause any serious bugs per se, but having an empty column in the beginning of the document looks weird to external users.
I propose adding a check on the value of $col before appending to the $columns array in FeedsCSVParser->getTemplate() ,
if (!empty(trim($col))) {
$columns[] = $col;
}
Patch coming. Take it or leave it. I just have a policy of filing an issue and patch if I need to modify a contrib module.
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff-2834026-10-11.txt | 5.58 KB | MegaChriz |
#11 | feeds-empty-template-columns-2834026-11.patch | 5.7 KB | MegaChriz |
| |||
#11 | 2834026-import-page.png | 26.48 KB | MegaChriz |
#10 | feeds-empty_template_columns-2834026-10.patch | 599 bytes | bburg |
| |||
#5 | feeds-empty_template_columns-2834026-5.patch | 599 bytes | bburg |
Comments
Comment #2
bburgPatch
Comment #4
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedTests are failing because prior to PHP 5.5, the function
empty()
could only be used on variables.http://php.net/empty
The documentation page of the
empty()
function suggests to usetrim($name) == FALSE
instead:Comment #5
bburgGood catch, thanks!
Here's the new patch.
Comment #6
bburgComment #8
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedSorry for maybe a bit of confusion. The column should be added when it is *not* empty. Now only empty columns are added ;).
trim($name) == FALSE
== empty.Comment #9
bburgDoh! That's what I get for not testing locally first. I'll push another in a bit.
Comment #10
bburgComment #11
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedI noticed one issue with the patch. If you had a column name called
0
in the source mappings, that zero wouldn't end up in the CSV template either.Additionally, when using a space as column name and used that as unique, the texts looked a bit weird on the import page:
So this called for additional tests. See attached patch for that. To fix the issue with
0
, I usedstrlen(trim($col))
instead.Comment #13
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedCommitted #11. Thanks, bburg!