search

djaodjin / djaodjin-saas

Django application for software-as-service and subscription businesses
Other
564 stars 124 forks source link

Add the ability to download CSV exports of subscriber lists. #20

Closed rene-armida closed 9 years ago

rene-armida commented 9 years ago

I haven't done anything to authenticate users to this view, but from my understand, I don't think that's necessary at this level? Isn't that handled separately?

smirolo commented 9 years ago

Correct, auth and access control is handled upfront elsewhere in the system.

It is a little messy to use the APIView (derived from djangorestframework) for accessing their queryset method. Previously we would have factor the get_queryset method into Mixins but it might be worth investigating Custom QuerySet in Django 1.7.

The code works. I am merging the pull request. Implementation can be cleaned up later.

Thanks.

rene-armida commented 9 years ago

Yes, it was definitely a bit of a gymnastic approach to just get a queryset. I think a better way would be to move the queryset itself to a model manager method, and then to have the JSON API views and the CSV views call that. Since I have a few more of these CSV views to make, I'll tackle that next. Does that sound good, @smirolo?

smirolo commented 9 years ago

Perfect. The methods might actually be on a custom queryset and use Django 1.7 manager auto-create feature.