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

Export to CSV #371

Merged
merged 8 commits into from
Jun 15, 2023
Merged

Export to CSV #371

merged 8 commits into from
Jun 15, 2023

Conversation

pandeymangg
Copy link
Contributor

What does this PR do?

Fixes #253

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change adds a new database migration
  • This change requires a documentation update

How should this be tested?

  • Test A
  • Test B

Checklist

  • Read the contributing guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran pnpm build
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues
  • Updated the Formbricks Docs if changes were necessary

@vercel
Copy link

vercel bot commented Jun 14, 2023

@pandeymangg is attempting to deploy a commit to the formbricks Team on Vercel.

A member of the Team first needs to authorize it.

@jobenjada jobenjada requested review from jobenjada and mattinannt and removed request for jobenjada June 14, 2023 14:49
Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pandeymangg Thanks a lot for your work and the PR 💪😊
I am having trouble getting my browser (Chrome as well as Firefox) to download the CSV file due to this browser error after clicking the "Export to CSV" button:

You are not allowed to load local resource: blob:nodedata:aaa52f51-f541-485f-897b-26824ae55e70

Johannes gets the same error. Can you reproduce this?
According to chatGPT, this is due to a security restriction in modern browsers to prevent downloading from the local file system, and it suggests creating a blob of type csv first and then downloading it.
Once this is fixed I can check the PR and CSV results properly 💪.

@pandeymangg
Copy link
Contributor Author

@mattinannt I'm not able to reproduce the same error in my browsers, neither chrome nor firefox. And I have actually created the blob of type csv first before making the URL object, I'm not sure what the exact problem is, I'll try to debug this and update the PR

@jobenjada
Copy link
Member

@mattinannt The CSV download is fixed and working well. No more remarks from my end, please merge when you agree.

Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pandeymangg thanks a lot for the changes 😊 The CSV now works great on my end, too 💪
The code also looks great :-)
I would like you to make some minor improvements to the exported CSV before merging the changes.


can you please use a dynamic file-name for the survey. e.g. the export of the survey named My Survey should be my_survey_responses_2023_06_15.csv

In this filename:

my_survey is a URL-friendly version of "My Survey". Spaces are replaced with underscores or hyphens, and all letters are lowercased.
responses specifies what kind of data is in the file.
2023_06_15 is the date when the file was created. This can be especially useful if there are multiple exports over time.


You currently have a Date Submitted submitted column in the CSV but for a more detailed data analysis it would be helpful to have the date-time. Can you please change the name of the column to Timestamp and the Date to an ISO-String?

@pandeymangg
Copy link
Contributor Author

@mattinannt Got it, I'll make these changes and update the PR.

Apart from these changes, I was also considering of changing the package that I'm using to convert json to csv. I'm currently using the json2csv/plainjs package, but I think a better alternative could be json2csv/node because all the conversion from json to csv is happening on the server only. What do you say?

Reference: https://juanjodiaz.github.io/json2csv/#/parsers/node-async-parser

@mattinannt
Copy link
Member

@mattinannt Got it, I'll make these changes and update the PR.

Apart from these changes, I was also considering of changing the package that I'm using to convert json to csv. I'm currently using the json2csv/plainjs package, but I think a better alternative could be json2csv/node because all the conversion from json to csv is happening on the server only. What do you say?

Reference: https://juanjodiaz.github.io/json2csv/#/parsers/node-async-parser

Thanks for pointing this out. I think switching to json2csv/node is a good idea since it doesn't block the javascript event loop.

Copy link
Member

@mattinannt mattinannt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pandeymangg Awesome, works great 💪 Thanks for this contribution; I'm going to merge the feature now 😊

@mattinannt mattinannt merged commit 6689131 into formbricks:main Jun 15, 2023
0 of 2 checks passed
@pandeymangg pandeymangg deleted the feat/csv-conversion branch June 15, 2023 12:03
waseemrabani pushed a commit to LuminosoInsight/formbricks that referenced this pull request Feb 2, 2024
* add CSV export feature to responses page
waseemrabani pushed a commit to LuminosoInsight/formbricks that referenced this pull request Feb 2, 2024
* add CSV export feature to responses page
waseemrabani pushed a commit to LuminosoInsight/formbricks that referenced this pull request Feb 2, 2024
* add CSV export feature to responses page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Download option for user responses
3 participants