OCD model migration #240

fgregg · 2019-04-15T18:51:27Z

This PR changes django-councilmatic to use opencivicdata models.

In concert with datamade/chi-councilmatic#252, these code changes can largely reproduce the Chicago Councilmatic site.

TODO:

signals

fgregg · 2019-05-13T20:20:43Z

@hancush this is ready for me to indoctrinate you.

hancush · 2019-06-12T23:06:26Z

@fgregg making good progress here.

can you say more about:

add order_by to meta class on EventAgendaItem
figure out how we want to handle updating the index

?

hancush · 2019-06-17T14:09:48Z

re: add order_by to meta class on EventAgendaItem, there's already clean_agenda_items which orders and dedupes agenda items.

django-councilmatic/councilmatic_core/models.py

Lines 711 to 721 in 8d64575

    
           @property 
        
           def clean_agenda_items(self): 
        
               agenda_items = self.agenda_items.order_by('order').all() 
        
               agenda_deduped = [] 
        
               descriptions_seen = [] 
        
               for a in agenda_items: 
        
                   if a.description not in descriptions_seen: 
        
                       descriptions_seen.append(a.description) 
        
                       agenda_deduped.append(a) 
        
               return agenda_deduped

we already use this property to display agenda items. should be able to continue using with minor changes, e.g., i think it's event.agenda to get the agenda items (not event.agenda_items).

fgregg · 2019-06-17T14:13:19Z

if you add ordering to the model class, it will, by default return querysets in that order. This is just an opportunity to clean up some code, and not required for this migration.

https://docs.djangoproject.com/en/2.2/ref/models/options/#ordering

hancush · 2019-06-18T16:54:48Z

Change log

Description

tl;dr - Remove the need for extensive and error-prone ETL by basing the Councilmatic app off Open Civic Data models, i.e., a jurisdiction-specific database fed by the scrapers, rather than maintaining a remote database containing a set of similar, Councilmatic-specific models.

There is a Python package containing Django versions of the OCD models. The relevant ones live in opencivicdata.core.models and opencivicdata.legislative.models.

Approach

Extend Open Civic Data models

Subclassing

Leverage multi-table inheritance to add additional fields to OCD models. The primary use case in the refactor is adding slugs to first-class models – Person, Event, Bill, and Organization - and adding a headshot to Person.

Since the scrapers create OCD objects, we added signals to each of the subclassed models to create the related Councilmatic model on save. Read more on Django signals. »

We have also added a management command to import headshot files.

Proxying

We make extensive use of proxy models to add custom managers and additional properties and methods to classes for use in Councilmatic code. This is an issue because the proxied models return OCD objects, not Councilmatic objects. To get around that, we used the hacky but effective django-proxy-overrides library to force models to return related objects as Councilmatic models, as needed.

To customize the model of related objects, proxy a model like opencivicdata.legislative.models.BillAction -> councilmatic_core.models.BillAction, and override one or more related object attributes with a ProxyForeignKey.

A brief summary of alternative approaches and why we ultimately did not choose to use them can be found here.

Related changes

Update attribute references
Handle changes in data type, e.g., cast date strings to datetime objects
Refactor use of obsolete raw SQL, e.g., management commands

Upgrade to Django 2

Update some module import paths
staticfiles -> static template tag
Build URLs via static template tag

Repair and extend the tests

Centralize fixtures and add a management command to regenerate them from a local instance DB
Align individual fixtures with the model change

Remaining changes

Update the councilmatic-starter-template to use the refactored release
- To that end, see Migrate Chicago to Open Civic Data models django-councilmatic. chi-councilmatic#252 (comment) for startup documentation
Add a management command to import shapes
- It can repurpose or crib heavily from the relevant portion of the obsolete import_data command
Repair the notifications app

fgregg

overall looks great! there are some thing would be nice to change. I can't mark this as a request change.

fgregg · 2019-06-18T20:53:21Z

councilmatic_core/feeds.py

@@ -120,7 +120,7 @@ def description(self, obj):
        return "Recent sponsored bills from " + obj.name + "."

    def items(self, person):
-        sponsored_bills = [s.bill for s in person.primary_sponsorships.order_by('-_bill__last_action_date')[:10]]
+        sponsored_bills = sorted((s.bill for s in person.primary_sponsorships), key=lambda x: x.last_action_date)[:10]


would be ideal to push this sorting to the query.

fgregg · 2019-06-18T20:54:41Z

councilmatic_core/haystack_indexes.py

+            last_date = timezone.now() - timedelta(36500)
+
+        if last_date:
+            last_date = last_date - last_date.utcoffset()


what's going on here? a comment on the method would be good.

i'll be honest with you, i have no idea

i think that an easier question to answer is: how should the index handle bills without a last action date?

it seems like this is placing them way, way in the past. is this for the legislative session filter? if so, then it would have the effect of not returning bills with this funny date for any of the current legislative sessions. i think that we could achieve this by simply returning None if there is no last_action_date, no?

i think that this has to do with https://github.com/datamade/chi-councilmatic/blob/master/chicago/search_indexes.py#L20-L26

but we should just handle that kind of changes in that code, and we override this method anyway. I think we can drop this method altogether.

done! rebuilt the chicago index to make sure it didn't break anything, and it looks ok to me!

fgregg · 2019-06-18T20:58:18Z

councilmatic_core/management/commands/convert_attachment_text.py


    def convert_document_to_plaintext(self):
+        import textract


why is this import here?

so you can run the tests w/o needing to install textract, which is a pita. https://textract.readthedocs.io/en/stable/installation.html / #193 (comment)

please add a comment to that effect.

fgregg · 2019-06-18T20:59:22Z

councilmatic_core/management/commands/make_fixtures.py

+from django.db import connection
+
+
+class Command(BaseCommand):


fgregg · 2019-06-18T21:57:21Z

councilmatic_core/models.py


+    bill = ProxyForeignKey(Bill, related_name='sponsorships', on_delete=models.CASCADE)
+    person = ProxyForeignKey(Person, null=True, on_delete=models.SET_NULL)


An organization can be a sponsor.

fgregg

approve

fgregg added 8 commits November 17, 2018 09:28

reset migrations

f3fe835

update django, start to OCD the models

b337366

council members coming into view

ba7766c

nearly pages working except about and search

9c088f6

bring back old migrations

63ba9e2

search and about page

adb584e

flake8, all pages working in Chicago

ff7c696

still using dateutil

316f6f5

This was referenced Apr 15, 2019

Opencivicdata migration checklist datamade/chi-councilmatic#249

Closed

Migrate Chicago to Open Civic Data models django-councilmatic. datamade/chi-councilmatic#252

Merged

Open Civic Data model migration #241

Closed

fgregg and others added 12 commits April 17, 2019 11:05

signals

2e06e07

Merge pull request #242 from datamade/ocd_signals

e27b618

signals

Merge branch 'ocd' of github.com:datamade/django-councilmatic into ocd

6fe938f

restore old migrations

240d124

fix up on_delete in older migrations for django 2.0

fc9bd86

comment out proxy related fields

e507f84

migrate the councilmatic_core tables

30503de

try to get travis tests working

9896aea

install flak8 for tests

f3b624f

reduce changes

d86dc1b

remove incremental haystack update

f23cd63

flake8 coming

1de8113

fgregg added 4 commits May 13, 2019 15:31

not using activity stream right now

a6dd9a4

password_reset is part of django_notifications app:

49525d8

__init__.py in signals folder

0dd2577

postgis on travis

14a7c58

fgregg force-pushed the ocd branch from 96fc9e7 to 14a7c58 Compare May 13, 2019 21:01

try relative path

1dff240

hancush added 4 commits June 12, 2019 17:03

Make the fixtures independent of one another

9849ea3

Remove import_data

f4c75b0

Refactor convert_rtf to use ORM

c4ffeac

take8

f0735d5

Add use_template kwarg

00b08ef

hancush added 5 commits June 18, 2019 12:34

Put the headshots in their own directory

abab18f

Collect static at the end of update_headshots

8c595a3

Update testing instructions

d71478e

Format code blocks

99f681c

Yet more formatting

50141df

hancush force-pushed the ocd branch from 9583fe1 to 50141df Compare June 18, 2019 18:29

hancush added 3 commits June 18, 2019 13:29

Add missing migration

fd66e3c

OCD_CITY_COUNCIL_ID -> OCD_CITY_COUNCIL_NAME

373a77b

Update test settings

6c2a9bd

fgregg commented Jun 18, 2019

View reviewed changes

hancush added 2 commits June 19, 2019 09:37

Drop ambiguous last action date preparation

8515994

Move ordering into primary_sponsorship property, add comments

4436df9

fgregg commented Jun 19, 2019

View reviewed changes

hancush added 3 commits June 19, 2019 10:55

Override BillSponsorship.organization

74e7555

Flake it up

1cd01eb

Handle sponsorships of bills with no last action date

449ff74

This was referenced Jun 24, 2019

Create management command for importing shapefiles #248

Closed

Create management command for importing Post shapes #249

Merged

hancush mentioned this pull request Jun 26, 2019

Update starter template to reflect changes in 1.0 datamade/councilmatic-starter-template#19

Closed

hancush merged commit d78d3d9 into 1.0 Jun 27, 2019

hancush deleted the ocd branch January 15, 2020 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCD model migration #240

OCD model migration #240

fgregg commented Apr 15, 2019 •

edited by hancush

fgregg commented May 13, 2019

hancush commented Jun 12, 2019

hancush commented Jun 17, 2019 •

edited

fgregg commented Jun 17, 2019

hancush commented Jun 18, 2019 •

edited

fgregg left a comment

fgregg Jun 18, 2019

fgregg Jun 18, 2019

hancush Jun 18, 2019

hancush Jun 18, 2019

fgregg Jun 19, 2019

hancush Jun 19, 2019

fgregg Jun 18, 2019

hancush Jun 18, 2019

fgregg Jun 18, 2019

fgregg Jun 18, 2019

fgregg Jun 18, 2019

fgregg left a comment


		bill = ProxyForeignKey(Bill, related_name='sponsorships', on_delete=models.CASCADE)
		person = ProxyForeignKey(Person, null=True, on_delete=models.SET_NULL)

OCD model migration #240

OCD model migration #240

Conversation

fgregg commented Apr 15, 2019 • edited by hancush

fgregg commented May 13, 2019

hancush commented Jun 12, 2019

hancush commented Jun 17, 2019 • edited

fgregg commented Jun 17, 2019

hancush commented Jun 18, 2019 • edited

Change log

Description

Approach

Extend Open Civic Data models

Subclassing

Proxying

Related changes

Upgrade to Django 2

Repair and extend the tests

Remaining changes

fgregg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgregg left a comment

Choose a reason for hiding this comment

fgregg commented Apr 15, 2019 •

edited by hancush

hancush commented Jun 17, 2019 •

edited

hancush commented Jun 18, 2019 •

edited