Skip to content
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

2.5 models, views, templates and ancillary files #510

Merged
merged 8 commits into from
Nov 26, 2019
Merged

2.5 models, views, templates and ancillary files #510

merged 8 commits into from
Nov 26, 2019

Conversation

fgregg
Copy link
Collaborator

@fgregg fgregg commented Nov 20, 2019

Overview

This PR makes changes to the models, views, templates, urls and ancillary files for the councilmatic 2.5 upgrade. This PR does not include the map view for the board members page, which will come in a separate PR.

Notes

This PR depends on changes to the scrapers and django-councilmatic. I believe they can be reviewed separately

Also, we are bit out of synch with LA metro master, we should discuss how we want to handle that.

Testing Instructions

Move councilmatic/settings_deployment.py.example to councilmatic/settings_deployment.py.

docker-compose up scrapers
docker-compose run --rm -e DJANGO_MANAGEPY_MIGRATE=off app python manage.py migrate
convert_attachment_text
docker-compose run --rm -e DJANGO_MANAGEPY_MIGRATE=off app python manage.py update_index --batch-size=100
docker-compose up

@fgregg fgregg requested a review from hancush November 20, 2019 17:04
Copy link
Collaborator

@hancush hancush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of great work in here, @fgregg. Thank you!

Several of the inline comments are related to a more robust Membership manager. I think this could help reduce duplicate code and make accessing subsets of memberships a lot easier to perform and read. LMK if you want to chat more about that.

Very excited to see what you've accomplished separately with the map and management commands.

councilmatic/settings.py Outdated Show resolved Hide resolved
councilmatic/settings_jurisdiction.py Show resolved Hide resolved
docker-compose.yml Outdated Show resolved Hide resolved
docker-compose.yml Show resolved Hide resolved
docker-entrypoint.sh Show resolved Hide resolved
lametro/views.py Show resolved Hide resolved
lametro/views.py Show resolved Hide resolved
lametro/views.py Outdated Show resolved Hide resolved
lametro/views.py Show resolved Hide resolved
fgregg and others added 2 commits November 25, 2019 13:33
Co-Authored-By: Hannah Cushman <hancush@users.noreply.github.com>
@fgregg
Copy link
Collaborator Author

fgregg commented Nov 25, 2019

@hancush thank you for the review! I've addressed some of your very good suggestions. Others, at your approval, I would like to defer to other PRs.

I do think that we should resolve the infelicities around pre-fetching in this PR, but I require your more detailed guidance.

Copy link
Collaborator

@hancush hancush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to wrap this up for the day. With the exception of one outstanding thread (https://github.com/datamade/la-metro-councilmatic/pull/510/files#r350337543) You've addressed all of my comments, @fgregg. Thank you very, very much for your work and willingness to discuss and make changes! 🎊

Here's a quick summary of things to accomplish later:

@fgregg fgregg merged commit f12d47d into 2.5 Nov 26, 2019
@hancush hancush deleted the 2.5_mv branch March 27, 2020 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants