I have a CSV that contains employee records. It's generate from an Excel file. The file's first column is a primary key that's a manually assigned integer value. For some reason a recent update started creating a blank record with the ID "0", even though the CSV file doesn't have a corresponding record. This Drupal record doesn't contain any valid data, including a blank entity title.

CommentFileSizeAuthor
#6 3230719.patch3.98 KBredwan jamous

Issue fork feeds-3230719

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

DamienMcKenna created an issue. See original summary.

damienmckenna’s picture

I noticed that the CSV contains a lot of blank records with no data, just rows of commas like this:

,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,
megachriz’s picture

I think it kinda makes sense that when feeding empty data, empty records get created. Apparently, an empty record still passes validation: Feeds does execute $entity->validate(); before saving the entity.

If I'm correct, the CSV parser in Feeds does skip completely empty lines in the file (lines that also don't have commas on them). Maybe the parser should also skip lines that only consists of the configured separator (commas in this case)?

Patches/issue forks are welcome.

redwan jamous’s picture

Assigned: Unassigned » redwan jamous
Related issues: +#2926060: CSV parser import additional blank entry

I will try creating a patch that treats lines with delimiter characters only as empty lines...

redwan jamous’s picture

StatusFileSize
new3.98 KB

Create a MR that attempts to solve the problem by adding the delimiter character to the list of trimmed characters in the next() method.

I think we can also make this configurable and disabled by default to maintain backwards compatibility. Would appreciate some thoughts on this.

Attaching a static patch file to use with composer...

redwan jamous’s picture

Assigned: redwan jamous » Unassigned
Status: Active » Needs review
megachriz’s picture

Status: Needs review » Fixed

This looks good to me. Thanks for fixing, @redwan jamous!

I've scheduled the merge.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.