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: Subscribers import can fail to load last line #6865

Closed
cobbspur opened this issue May 19, 2016 · 6 comments · Fixed by #6923
Closed

Bug: Subscribers import can fail to load last line #6865

cobbspur opened this issue May 19, 2016 · 6 comments · Fixed by #6923
Assignees
Labels
bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with

Comments

@cobbspur
Copy link
Member

Issue Summary

During a subscribers CSV import, if the CSV file does not end with a newline, the final line is not included.

Steps to Reproduce

Make a column of a few email address in Google sheets.
Download as CSV (do not modify file - many editors automatically add a line)
Import CSV on /ghost/subscribers/

Result is 1 less imported email address than the CSV

@kirrg001
Copy link
Contributor

It only affects CSV files without column headers. Can you confirm?
If so, i have a fix in mind and will care.

@cobbspur
Copy link
Member Author

cobbspur commented May 20, 2016

No, If there are no column headers you get the error - Column header missing: "email"

The readline module looks for a new line - (\r, \n, \r\n) before returning 'line' which means the read stream ends but the final line is not returned. We could perhaps read the whole stream first and check/append a new line at the end then read this line by line.

@sebgie sebgie added the bug [triage] something behaving unexpectedly label May 20, 2016
@sebgie
Copy link
Contributor

sebgie commented May 20, 2016

FYI: nodejs/node-v0.x-archive#7238 it's a bug in Node 0.10, fixed in 0.12 and there is a suggestion for a workaround.

@kirrg001
Copy link
Contributor

Thanks @sebgie
In my opinion this is a low priority bug, because 1. nothing crashes and the feature still works 2. only 0.10 is affected 3. only one subscriber is not imported

If somebody is interested in this bug fix, pick it up 👍

@ErisDS ErisDS added the help wanted [triage] Ideal issues for contributors to help with label May 22, 2016
@cobbspur
Copy link
Member Author

👍 confirmed issue is resolved on node v4

@ErisDS
Copy link
Member

ErisDS commented May 23, 2016

Node 0.10 is still our recommended version and the version running on Ghost(Pro). Although that will be changing soon, I'd like to see a workaround for this go into the next release.

cobbspur added a commit to cobbspur/Ghost that referenced this issue Jun 7, 2016
closes TryGhost#6865

- switch csv-read to use a csv-parser for greater reliability and management of strings when importing a csv
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this issue Nov 20, 2016
closes TryGhost#6865

- switch csv-read to use a csv-parser for greater reliability and management of strings when importing a csv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [triage] something behaving unexpectedly help wanted [triage] Ideal issues for contributors to help with
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants