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

(I think I) found a bug #65

Closed
pietkamps opened this issue Apr 4, 2020 · 6 comments · Fixed by #67
Closed

(I think I) found a bug #65

pietkamps opened this issue Apr 4, 2020 · 6 comments · Fixed by #67
Assignees
Labels
bug ready to close Issue reporter will close when satisfied test required
Milestone

Comments

@pietkamps
Copy link

I think i found a bug. When i run the following code:

var jsonexport = require('jsonexport');
var invited =
[
{
day : "1",
time : "12:00",
persons :
[
{name: 'Bob',
lastname: 'Smith'},
{name: 'James',
lastname: 'piet'},
{name: "",
lastname: 'Miller'},
{name: 'David',
lastname: 'Martin'}
]
}
]
const options = {fillGaps: true};
jsonexport(invited, options, function (err, csv) {
if (err) return console.log(err);
console.log(csv);
});

The output is:
day,time,persons.name,persons.lastname
1,12:00,Bob,Smith
1,12:00,James,piet
1,12:00,David,Miller
1,12:00,David,Martin

As you can see the third object has an empty string "". But in the output the name "David" is placed. What i expect of course is an empty string.

I hope the explanation is clear.
Thank you in advance,
Piet Kamps

@cbschuld
Copy link

cbschuld commented Apr 6, 2020

@pietkamps - just downloaded jsonexport myself and encountered the exact same bug. Specifically if it is an empty value.

@max-bruhn
Copy link

max-bruhn commented Apr 19, 2020

i have encountered the same bug with my empty strings.
#41 (comment)

@AckerApple
Copy link
Collaborator

AckerApple commented Apr 19, 2020

An online demo tool exists for jsonexport and I used it to check the mentioned data call outs.

Online tool link: https://kauegimenes.github.io/jsonexport/demo/

Here is valid JSON for this tickets mentioned issue. It does NOT convert to CSV correctly (bug confirmed)

[
    {
        "day": "1",
        "time": "12:00",
        "persons": [
            {
                "name": "Bob",
                "lastname": "Smith"
            },
            {
                "name": "James",
                "lastname": "piet"
            },
            {
                "name": "",
                "lastname": "Miller"
            },
            {
                "name": "David",
                "lastname": "Martin"
            }
        ]
    }
]

Invalid result line 3 and 4:

day,time,persons.name,persons.lastname
1,12:00,Bob,Smith
,,James,piet
,,David,Miller
,,,Martin

Expected Result for line 3 and 4:

day,time,persons.name,persons.lastname
1,12:00,Bob,Smith
,,James,piet
,,,Miller
,,David,Martin

I don't know when but I will attempt a fix in the near future.

Do you want a fix faster? This code is not that difficult, participate, make a pull request.

NOTE TO SELF: Upgrade demo web page. Don't require just JSON and use eval to cast javascript vars (consider security cost of running eval).

@kaue
Copy link
Owner

kaue commented May 23, 2020

@pietkamps I finally got a little time to catch up with jsonexport :)
I just opened a PR that fixes this issue #67
@AckerApple can you do a code review?

@kaue kaue added this to the 3.0.0 milestone May 23, 2020
@kaue kaue mentioned this issue May 23, 2020
3 tasks
@AckerApple
Copy link
Collaborator

Fix released as version 2.5.0

Please acquire. Please note we are going to a version 3.0.0 soon

@AckerApple AckerApple added the ready to close Issue reporter will close when satisfied label May 25, 2020
@AckerApple
Copy link
Collaborator

NEW: You should be able to jump to this url to see proof of this fix

@kaue kaue closed this as completed in 6965655 May 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ready to close Issue reporter will close when satisfied test required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants