Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: DataSet::parseData() disregards valid content due to empty check #488

Closed
alexanderschnitzler opened this issue Aug 7, 2023 · 3 comments · Fixed by #489
Closed

BUG: DataSet::parseData() disregards valid content due to empty check #488

alexanderschnitzler opened this issue Aug 7, 2023 · 3 comments · Fixed by #489

Comments

@alexanderschnitzler
Copy link
Contributor

alexanderschnitzler commented Aug 7, 2023

Hi,

in one of my tests I need to import sys_file_metadata fixtures. As said table, does not have a uid column, my csv looked as such:

"sys_file_metadata",
,"pid","file"
,0,1
,0,2

I realized that my fixtures aren't imported due to an empty-check in the parser:

} elseif ($tableName !== null && !empty($values[1])) {

I guess it would be better to compare against an empty string here because "0" is considered empty but it is a valid value.

Have a good one.


Related to #451

@sbuerk
Copy link
Collaborator

sbuerk commented Aug 7, 2023

That's basically the same spot like reported for the assertCSVData()
in #451.

@lolli42
Copy link
Member

lolli42 commented Aug 8, 2023

I don't get it: sys_file_metadata is a casual TCA table and very well has uid column?!

@alexanderschnitzler
Copy link
Contributor Author

alexanderschnitzler commented Aug 8, 2023

Just to complete the picture here.

Yes, said table has a uid but it's not under test in my case, therefore I don't care what uid the records have. Only interesting column is file because I do check relations to files.

However, that's not the point here. A silently failing parser, that drops syntactically correct csv data, is. That's the issue for my customer. Me digging half an hour or more into this for nothing. Please remember, people have to pay our work and as hard as TDD is to sell, this is just unnecessary water to the mills for the "we don't pay tests" faction.

oliverklee added a commit to oliverklee/typo3-testing-framework that referenced this issue Aug 9, 2023
A DB column value "0" is a perfectly valid value and should not be
considered to be empty.

Particularly, having a "0" value as the first value in a DB row
in a CSV file should still allow the DB row to get importet.

(In general, `empty()` in PHP behaves in mysterious ways and should
be avoided in favor of more explicit and human-predicable checks.)

Fixes TYPO3#488

Releases: main, 7, 6
lolli42 pushed a commit that referenced this issue Aug 12, 2023
A DB column value "0" is a perfectly valid value and should not be
considered to be empty.

Particularly, having a "0" value as the first value in a DB row
in a CSV file should still allow the DB row to get importet.

(In general, `empty()` in PHP behaves in mysterious ways and should
be avoided in favor of more explicit and human-predicable checks.)

Fixes #488

Releases: main, 7, 6
lolli42 pushed a commit that referenced this issue Aug 12, 2023
A DB column value "0" is a perfectly valid value and should not be
considered to be empty.

Particularly, having a "0" value as the first value in a DB row
in a CSV file should still allow the DB row to get importet.

(In general, `empty()` in PHP behaves in mysterious ways and should
be avoided in favor of more explicit and human-predicable checks.)

Fixes #488

Releases: main, 7, 6
lolli42 pushed a commit that referenced this issue Aug 12, 2023
A DB column value "0" is a perfectly valid value and should not be
considered to be empty.

Particularly, having a "0" value as the first value in a DB row
in a CSV file should still allow the DB row to get importet.

(In general, `empty()` in PHP behaves in mysterious ways and should
be avoided in favor of more explicit and human-predicable checks.)

Fixes #488

Releases: main, 7, 6
lolli42 pushed a commit that referenced this issue Aug 12, 2023
A DB column value "0" is a perfectly valid value and should not be
considered to be empty.

Particularly, having a "0" value as the first value in a DB row
in a CSV file should still allow the DB row to get importet.

(In general, `empty()` in PHP behaves in mysterious ways and should
be avoided in favor of more explicit and human-predicable checks.)

Fixes #488

Releases: main, 7, 6
lolli42 pushed a commit that referenced this issue Aug 12, 2023
A DB column value "0" is a perfectly valid value and should not be
considered to be empty.

Particularly, having a "0" value as the first value in a DB row
in a CSV file should still allow the DB row to get importet.

(In general, `empty()` in PHP behaves in mysterious ways and should
be avoided in favor of more explicit and human-predicable checks.)

Fixes #488

Releases: main, 7, 6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants