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

Display local council boundaries and add some basic council data #506

Merged
merged 59 commits into from
Apr 24, 2024

Conversation

struan
Copy link
Member

@struan struan commented Mar 13, 2024

This adds local council boundaries to the map along with imports for the population and the type so there is some data there. It also makes minimal changes to the area page for local councils.

@struan struan requested a review from zarino March 13, 2024 11:11
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 73.56322% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 82.11%. Comparing base (3f18e33) to head (165bd4d).
Report is 8 commits behind head on main.

❗ Current head 165bd4d differs from pull request most recent head f983cff. Consider uploading reports for the commit f983cff to get more accurate results

Files Patch % Lines
hub/views/area.py 70.00% 7 Missing and 2 partials ⚠️
hub/mixins.py 0.00% 5 Missing and 2 partials ⚠️
hub/management/commands/base_importers.py 50.00% 3 Missing ⚠️
hub/templatetags/hub_filters.py 66.66% 2 Missing ⚠️
hub/views/explore.py 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
- Coverage   82.50%   82.11%   -0.39%     
==========================================
  Files         104      105       +1     
  Lines        3440     3483      +43     
  Branches      347      359      +12     
==========================================
+ Hits         2838     2860      +22     
- Misses        515      532      +17     
- Partials       87       91       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@zarino zarino left a comment

Choose a reason for hiding this comment

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

Yep, Area page and Explore page both work for councils 👍

Have suggested a few improvements.

hub/management/commands/import_areas.py Outdated Show resolved Hide resolved
@@ -120,9 +120,11 @@ <h3 class="h5"><a href="{% url 'area' area_type=overlap_constituency.old_area.ar
</a>
</li>
{% endif %}
{% if is_westminster_cons %}
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice if this was two-space indented, like the conditional above ;-)

(I try to always do this inside Django templates, as per the "suggestion" in https://pages.mysociety.org/coding-standards.html)

@struan struan requested a review from zarino March 19, 2024 15:42
@struan struan force-pushed the 487-display-local-councils branch 3 times, most recently from 0d146c4 to 165bd4d Compare April 2, 2024 15:44
Copy link
Member

@zarino zarino left a comment

Choose a reason for hiding this comment

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

Overall looks good 👍 Two small things:

There’s too much vertical space above the public opinion section, on council area pages, because the (empty) MP section is still there in the markup. Any way we could skip printing that section entirely, if we’re on a council page?

Screenshot 2024-04-08 at 13 58 18

And is there any way we can get a filter in the DataSet admin, to filter by council type(s)?

Screenshot 2024-04-08 at 13 58 33

@zarino zarino linked an issue Apr 23, 2024 that may be closed by this pull request
@struan struan force-pushed the 487-display-local-councils branch 2 times, most recently from 00793df to 06ec6ad Compare April 24, 2024 14:07
struan and others added 18 commits April 24, 2024 16:06
Corrects headers according to what's displayed.
Sorts by Council or Constituency name as appropriate.
Make sure GSS codes can be displayed for councils.
There is no point repeating the dataset’s label in its description.
If the import script does not set a description, the dataset shouldn’t
have one.
Fixes #504.

AreaSearchView no longer artificially limits results to WMC area_types,
and area_search.html presents matching areas grouped by area_type.

I’ve gone with a custom `grid` layout on the area_search.html (rather
than Bootstrap columns) to more gracefully handle a variable number of
columns – there might be 1, 2, or 3, depending on the search input.

I also fixed a bug in the `highlight` filter, which raised a TypeError
when passed a None `search` parameter. `search` is None when the user
has used geolocation rather than submitting a search term.
-"select button" display the area name + description.
-"select button" Opens a modal displaying the area option with an description for each one of them.
-Button content is updated when user selects an option.
This prevents “MP” showing as an always-empty column when you’re
exploring by local authority.

I implemented this as an `areaTypeHasMP()` method, because
`selectableDatasets`` changes when you enter search text, and I don’t
want all of the columns to appear/disappear as you search.

While I was there, I also took the opportunity to refactor the four
separate “Loading datasets…” status messages into a single, centred one.
struan added 28 commits April 24, 2024 16:06
use a lookup for the cons column if present
move range display into include
allow a range related dataset for datasets
Not clear why it was ever using file.get to set the area type as it's
never had an area_type key. Changed to using the area_type of the
importer as that's the correct thing.
@struan struan force-pushed the 487-display-local-councils branch from f0e8006 to f983cff Compare April 24, 2024 15:07
@struan struan merged commit f983cff into main Apr 24, 2024
5 checks passed
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.

Make search handle multiple area types
3 participants