datamade / bga-payroll

💰 How much do your public officials make?
4 stars 4 forks source link

add csv export for processed salary data #527

Closed smcalilly closed 2 years ago

smcalilly commented 2 years ago

Overview

Code changes include:

Closes #515.

Notes

Any ideas where this link should exist on the webpage? It's not styled, currently.

Testing

fgregg commented 2 years ago

yep

On Wed, Aug 25, 2021 at 2:07 PM Sam McAlilly @.***> wrote:

@.**** commented on this pull request.

In payroll/views.py https://github.com/datamade/bga-payroll/pull/527#discussion_r695996646:

@@ -174,6 +175,46 @@ def get_context_data(self, **kwargs): return context

+class DownloadView(TemplateView):

  • def get(self, request, *args, **kwargs):
  • slug = request.GET.get('employer')
  • year = request.GET.get('year')
  • employer = Employer.objects.get(slug=slug)
  • employer_salaries = employer.get_salaries(year=year)
  • response = HttpResponse(content_type='text/csv')
  • filename = '{employer}-{year}.csv'.format(employer=employer.name, year=year) # noqa
  • response['Content-Disposition'] = 'attachment; filename="{}"'.format(filename) # noqa
  • writer = csv.writer(response)

are you suggesting that i put what i want in a dictionary and then use the DictWriter?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/datamade/bga-payroll/pull/527#discussion_r695996646, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDC3NLKVIXXHM3445HUBDT6UWMZANCNFSM5CXXIEDQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

smcalilly commented 2 years ago

How would you characterize the speed?

@fgregg Speed isn't the greatest. It just took 79 seconds to download 21k records: http://localhost:8000/unit/cook-county-5370fcc1/?data_year=2018.

The bottleneck is looping through each row to create the csv. Do you have any suggestions on making that faster? Or, should we add some indicator in the UI to let the user know that it's still downloading? A loading icon in the browser's tab is the only feedback that user gets (on firefox, at least). It's not the most prominent indicator. Otherwise there is no feedback about the loading status.

fgregg commented 2 years ago

How would you characterize the speed?

@fgregg Speed isn't the greatest. It just took 79 seconds to download 21k records: http://localhost:8000/unit/cook-county-5370fcc1/?data_year=2018.

The bottleneck is looping through each row to create the csv. Do you have any suggestions on making that faster? Or, should we add some indicator in the UI to let the user know that it's still downloading? A loading icon in the browser's tab is the only feedback that user gets (on firefox, at least). It's not the most prominent indicator. Otherwise there is no feedback about the loading status.

cool. there's a ton of things we can do to improve the speed. Let's get a basic implementation landed, and then we can work on speeding it up.

smcalilly commented 2 years ago

@fgregg i refactored the dict writer here: 343e134

commenting this as a reminder to self, don't merge until hannah's green light.

fgregg commented 2 years ago

can you fix the tests @smcalilly ?