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

fix button action export for csv #4911

Merged
merged 3 commits into from
Mar 16, 2022
Merged

fix button action export for csv #4911

merged 3 commits into from
Mar 16, 2022

Conversation

PClmnt
Copy link
Collaborator

@PClmnt PClmnt commented Mar 15, 2022

Description

CSV data wasn't being exported correctly, so added the csv headers and converted the data

@PClmnt PClmnt requested a review from aptkingston March 15, 2022 14:56
Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

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

This looks like it'll work just fine, but I think it would be better to do this on the server side like we do our other exports - the server already has the utilities needed. Check out the view controller for an example.

@PClmnt
Copy link
Collaborator Author

PClmnt commented Mar 16, 2022

@aptkingston Now returning the file from the backend

@codecov-commenter
Copy link

codecov-commenter commented Mar 16, 2022

Codecov Report

Merging #4911 (43f7d16) into master (a555659) will decrease coverage by 0.12%.
The diff coverage is 23.52%.

@@            Coverage Diff             @@
##           master    #4911      +/-   ##
==========================================
- Coverage   69.09%   68.97%   -0.13%     
==========================================
  Files         145      145              
  Lines        5019     5034      +15     
  Branches      769      769              
==========================================
+ Hits         3468     3472       +4     
- Misses       1096     1107      +11     
  Partials      455      455              
Impacted Files Coverage Δ
...ackages/server/src/api/controllers/row/external.js 16.36% <22.22%> (+0.67%) ⬆️
...ackages/server/src/api/controllers/row/internal.js 77.61% <25.00%> (-1.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65ea8e4...43f7d16. Read the comment docs.

Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

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

Nice one, definitely better doing it on the back end 👍

@PClmnt PClmnt merged commit c954886 into master Mar 16, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 16, 2022
@PClmnt PClmnt deleted the pc/csv-bug-fix branch October 25, 2022 09:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants