codeforamerica / ohana-api

The open source API directory of community social services.
http://ohana-api-demo.herokuapp.com/api
BSD 3-Clause "New" or "Revised" License
185 stars 344 forks source link

CSV export from Admin GUI Issue #437

Closed pabshazon closed 6 years ago

pabshazon commented 6 years ago

I am getting html files in .csv extension when I generate + export the .zip file with the .csv files inside from the Ohana Admin GUI as masteruser

monfresh commented 6 years ago

Thanks for the report! I can reproduce this and will look into it.

monfresh commented 6 years ago

Looks like something is wrong with the generation of the zip file. In the meantime, you can download each CSV file individually by accessing the desired table using a URL such as https://ohana-api-demo.herokuapp.com/admin/csv/locations.csv

monfresh commented 6 years ago

This is due to this change: https://github.com/codeforamerica/ohana-api/blob/master/app/controllers/admin/csv_controller.rb#L3-L4

I failed to test the contents of the CSV files when I made that change. I'll have to think about how to allow the zip file generation while also preventing people who are not super admin from being able to download all the data.

monfresh commented 6 years ago

Off the top of my head, I'm not sure how to do that, so what I think I will do is remove the zip file feature and replace it with buttons to download all the possible tables. Thoughts?

pabshazon commented 6 years ago

I see! Thanks for checking it out so fast and proposing a work around: it works!

About how to solve it for the non superusers to access it, I really don't know ruby on rails so cannot help you!

From a language/framework agnostic perspective, you could check the role or ACLs related to the user when the URL is called. I think rails is MVC, so maybe checking the user is logged as supersuer in the action in the controller before returning the file or an access denied?

monfresh commented 6 years ago

The problem is that the zip file is generated via a background job that makes a GET request to each table's endpoint (such as admin/csv/locations.csv), then fetches the response and writes it to a CSV file. Once all the CSV content has been fetched and written into individual CSV files, they get all zipped up into one file: https://github.com/codeforamerica/ohana-api/blob/master/lib/csv_downloader.rb#L13-L21

In order for the background job to fetch the CSV endpoints, it needs to have access, but it's not making the request as any type of user, and I'm not sure if there's a way to tell the app "hey, if the request is coming from the background job, allow it".

The zip file was a convenience feature so that you didn't have to download multiple files individually, but I think it might be better to have individual buttons. There have been issues with the zip file generation outside of this authentication bug. I think it's worth it to remove a lot of the complexity around the zip file generation and just have individual download links.

pabshazon commented 6 years ago

Understood. Yes, I think it makes sense the individual download links! Simple is good! :+1:

pabshazon commented 6 years ago

By the way, this is off-topic but I don't know where to ask... do you know of any Ohana API server with information on US cities?

We are populating one with San Francisco Data, but it would be awesome to aggregate info or have a repo of ohana api servers with info about other places.

greggish commented 6 years ago

@pabshazon I think the San Mateo Ohana API is the only other Ohana that's operated by a local government.

I'd like to learn more about your work with San Francisco data. Are you working with the city gov on this? We've got a workshop coming up there in a few weeks, it would be great to get in touch.

pabshazon commented 6 years ago

@greggish thanks for the link! We are not working with the city even thou we are talking to them in friendly collaboration. We are doing this felixnomad.com

Send me a link for the workshop please! my email is pablo@nomads.ai