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

Split single tier types #707

Merged
merged 4 commits into from
Nov 18, 2024
Merged

Split single tier types #707

merged 4 commits into from
Nov 18, 2024

Conversation

zarino
Copy link
Member

@zarino zarino commented Nov 15, 2024

Fixes #611.

Adds an "Advanced filter" for authority_type, on the Single Tier scorecard.

Screenshot 2024-11-15 at 18 50 39

While I was there, I also fixed a long-standing code smell, where we had been using authority_type to refer to both actual MapIt council authority types (eg: "UTA", "DIS", "CTY") and Scorecards scoring groups (eg: "single-tier").

This makes the PR look much more complex than it actually is. Best to review each commit separately!

@zarino zarino requested a review from struan November 15, 2024 18:53
Copy link
Member

@struan struan left a comment

Choose a reason for hiding this comment

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

Few small fixups added and one additional fix I noticed while in here but yup, looks good.

zarino and others added 4 commits November 18, 2024 15:18
…_type

Fixes #611.

Adds a new “Advanced filter” for “Single Tier authority type”, allowing
users to filter the homepage scorecard table down to just Metropolitan
Districts, for example, or just London Boroughs.
Just a small code quality improvement I spotted while implementing
the Single Tier authority_type filtering.

Use the group slugs and authority types in Council.SCORING_GROUPS in
Council.get_scoring_group() instead of repeating them manually. Less
chance of the two falling out of sync.
We previously used `authority_type` in a number of places, when really
we were referring to Scorecards scoring groups (eg: "Single Tier"), not
a council’s _actual_ MapIt `authority_type` as the name might imply. We
also sometimes used the term `council_group` for the same thing. Sigh.

This commit is an attempt to disambiguate all those `authority_type`
and `council_group` references, so that now, whenever we’re talking
about a Scorecards scoring group, we call it `scoring_group` and we pass
the group’s full object from Council.SCORING_GROUPS, not just its slug.

This involved a number of fiddly changes. including:

- Removing `caps/includes/authority_type.html` and using
  `scoring/includes/scoring-group-name.html` instead.
- Replacing `context["authority_type"]` with `context["scoring_group"]`
  in all post-2022 views and templates.
- Replacing `council_group` with `scoring_group` in `PlanSection.get_average_scores`
  and `PlanQuestion.get_scores_breakdown`.
- …And while I was there, I also noticed that `PlanQuestion.get_average_scores`
  wasn’t called anywhere, so I removed it.
Remove incorrect checking for None and just use the description
@zarino
Copy link
Member Author

zarino commented Nov 18, 2024

Thanks @struan! I spotted that very cludgey filter description appending behaviour but didn’t want to get even further into the weeds fixing it. Glad you did it instead of me! 🤣

@zarino zarino merged commit d1ca74f into master Nov 18, 2024
3 of 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.

[scorecards] update filtering to enable spliting metro and non metro districts
2 participants