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 accounting data to csv #127

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Export accounting data to csv #127

merged 3 commits into from
Sep 13, 2024

Conversation

hunxjunedo
Copy link
Contributor

@hunxjunedo hunxjunedo commented Aug 7, 2024

fixes #15

@hunxjunedo
Copy link
Contributor Author

Hey @tomlebl sorry for the inconvenience here, unfortunately I could not get it to install the package, seems an os-based issue. Therefore I want you to test out this PR's grants export in your environment before I can start the work on other exports.

@tomlebl
Copy link
Contributor

tomlebl commented Aug 12, 2024

@hunxjunedo I just came back from holidays and had a brief look. I see that you try to use react-make-csv library that has very small track record. It has only 33 weekly downloads. Using library could be source of many troubles.
You could possibly use something else react-csv (https://www.npmjs.com/package/react-csv) has over half million weakly downloads and thus it will likely be more solid.
I would use the CSVLink component. We could just simply render it at the bottom of the table every time the table is rendered. If the data manipulation would take too much resources we could go to the original plan and render the link on a button click. However, I don't think that would be necessary.

@hunxjunedo hunxjunedo marked this pull request as ready for review September 11, 2024 17:01
@hunxjunedo
Copy link
Contributor Author

@tomlebl took a little too much time, but it's finally completed, please have a lool

@tomlebl
Copy link
Contributor

tomlebl commented Sep 12, 2024

Great. I thought that you gave up. I will have a look at some point today or tomorrow

@hunxjunedo
Copy link
Contributor Author

I was being dumb the whole time, installing the package in local dir instead of the container 🤦

@tomlebl tomlebl merged commit 53401e0 into nomad-nmr:main Sep 13, 2024
1 check passed
@tomlebl
Copy link
Contributor

tomlebl commented Sep 13, 2024

@hunxjunedo I think that I see what was wrong now. What you did actually did not work out of the box in my environment due to the change that you did in vite.config.js. It took actually awhile to figure out why it was not working.
Anyway, if you have dev environment in Docker and want to add a new or change dependency, you just simply do standard npm install or you could also just edit package.json but then you have to rebuild the images docker-compose up -d --build
otherwise you keep using old images with old package-lock.json and /node_modules. The --build flag basicly triggers build of new images when npm install is repeated in the docker.
I hope that makes sense.
Otherwise, good job. I have done only minor design changes.

@hunxjunedo
Copy link
Contributor Author

How did it go in your environment? Idk why it's giving me an error: proptypes doesn't export a module named array. Did you come across this too ?

@hunxjunedo
Copy link
Contributor Author

I'm actually trying to do some improvements and optimizations on the backend, but the frontend gives this error.

@tomlebl
Copy link
Contributor

tomlebl commented Sep 13, 2024

That's exactly the error message that I was getting and could not figure it out until I noticed the change in vite.config.js. I reverted and that solved the problem. What I get from here https://vitejs.dev/config/dep-optimization-options is that the option excluded react-csv to be prebundled by vite which does not make sense to me.
It works at the end.

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.

Export accounting table as excel/.csv file
2 participants