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

Fixed issue: Survey participant data with quotes are not properly exported in CSV #2073

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

ViliusS
Copy link
Contributor

@ViliusS ViliusS commented Oct 4, 2021

Dev: Participant's name could contain quotes, for example if the participant is a company. Quotes could also be present in attribute fields. This fix allows to export quote characters in case these field contain them.

Dev: This ONLY fixes quotes, but there are also other characters like new lines, which still need to be properly escaped! The proper way to do CSV export would be to use fputcsv() and then redirect everything to php://output which automatically takes care of everything.

Dev: This is a backport to LTS-3.x from #2072

…orted in CSV

Dev: Participant's name could contain quotes, for example if the participant is a company. Quotes could also be present in attribute fields. This fix allows to export quote characters in case these field contain them.

Dev: This ONLY fixes quotes, but there are also other characters like new lines, which still need to be properly escaped! The proper way to do CSV export would be to use fputcsv() and then redirect everything to php://output which automatically takes care of everything.

Dev: This is a backport to LTS-3.x from LimeSurvey#2072
@glimz
Copy link
Contributor

glimz commented Nov 10, 2021

@ViliusS I cannot find any Mantis Bug report for this issue. Can you create one so we can test and merge your PR? Thank you

@glimz glimz added the Feedback Waiting for feedback from PR author label Nov 10, 2021
@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 10, 2021

Sorry I don't understand this requirement, nor do I have a Mantis account.

@glimz
Copy link
Contributor

glimz commented Nov 12, 2021

Hello @ViliusS , we cannot directly merge your PR without a bug report. You have to report a bug and provide sufficient details so our QA can reproduce the issue.

This will enable us properly track all PRs and bugs reported and improve the workflow for testing and merging new PR.

You can easily create an account and report a bug.

We will pick up the issue directly from mantis.

Thank you

@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 12, 2021

I understand you want me to create a bug report, but I don't understand WHY you need me to do this. Instead of just a report I have provided you with complete fix. All the information what this PR fixes is in the commit message.

@Shnoulle
Copy link
Collaborator

Not related to discussion : why we don't use fputcsv here ?

@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 13, 2021

Not related to discussion : why we don't use fputcsv here ?

As I have mentioned in the commit, yes, fputcsv() is the proper way to do this. However it needs almost full rewrite of the tokensExport() and could need a lot more testing, which is not ideal for LTS branch.

I will leave that to somebody else, who has more time. My patch is a quick fix with minimal risk.

@Shnoulle
Copy link
Collaborator

As I have mentioned in the commit, yes, fputcsv() is the proper way to do this. However it needs almost full rewrite of the tokensExport() and could need a lot more testing, which is not ideal for LTS branch.

I will leave that to somebody else, who has more time. My patch is a quick fix with minimal risk.

👍 must create a dev issue :).

@c-schmitz c-schmitz merged commit 1c3baeb into LimeSurvey:3.x-LTS Feb 25, 2022
@c-schmitz
Copy link
Contributor

Thank you!

luismontalba pushed a commit to luismontalba/LimeSurvey that referenced this pull request Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feedback Waiting for feedback from PR author
Projects
None yet
4 participants