-
Notifications
You must be signed in to change notification settings - Fork 79
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
Dump departmental data as CSV files #531
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very cool feature! a couple comments inline
OpenOversight/app/main/views.py
Outdated
assign_dict[r.officer_id] = [] | ||
assign_dict[r.officer_id].append("(#%s %s %s %s %s)" % (check_input(r.star_no), check_input(r.rank), check_input(r.unit), check_input(r.star_date), check_input(r.resign_date))) | ||
|
||
notes_dict = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove the notes from the CSV, the reason being is that the notes are actually for admins/area coordinators to add private notes to a profile, so we don't want to expose them to regular users
check_input(record.report_number), | ||
check_input(record.date), | ||
check_input(record.description), | ||
check_input(record.address), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a string representation for Location
so the output here is nice, i.e.:
diff --git a/OpenOversight/app/models.py b/OpenOversight/app/models.py
index c4b944b..9e87a13 100644
--- a/OpenOversight/app/models.py
+++ b/OpenOversight/app/models.py
@@ -222,6 +222,20 @@ class Location(db.Model):
def validate_state(self, key, state):
return state_validator(state)
+ def __repr__(self):
+ if self.street_name and self.cross_street2:
+ return 'Intersection of {} and {}, {} {}'.format(
+ self.street_name, self.cross_street2, self.city, self.state)
+ elif self.street_name and self.cross_street1:
+ return 'Intersection of {} and {}, {} {}'.format(
+ self.street_name, self.cross_street1, self.city, self.state)
+ elif self.street_name and self.cross_street1 and self.cross_street2:
+ return 'Intersection of {} between {} and {}, {} {}'.format(
+ self.street_name, self.cross_street1, self.cross_street2,
+ self.city, self.state)
+ else:
+ return '{} {}'.format(self.city, self.state)
+
<a class="btn btn-lg btn-primary" href="{{ url_for('main.download_dept_csv', department_id=department.id) }}"> | ||
Download Officer Data | ||
</a> | ||
</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary for merge on this PR, but just a thought: we should have the officers and incidents links be primary, and the raw data downloads be somewhere less central. One good place on the site might be a link on the footer to a new route that has the CSV download links for each department:
If you feel like implementing this (or if you have another better idea!) feel free, else we can defer to a followup.
b6350e9
to
b0d244e
Compare
dfcce6e
to
133152f
Compare
Thanks for the review @redshiftzero . I removed the notes from the CSV, added your Location representation, and changed the location of the download links to the footer: |
…dump_dept_csv_489
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Status
Ready for review / in progress
Description of Changes
Fixes #489
Changes proposed in this pull request:
Notes for Deployment
The final presentation and formatting of data in the CSVs is undecided and open to feedback. Better str methods in some of the model objects would improve presentation
Screenshots (if appropriate)
Tests and linting
I have rebased my changes on current
develop
pytests pass in the development environment on my local machine
flake8
checks pass