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

Migrate everything to the new statistics API / choropleth UX smoothing #213

Merged
merged 24 commits into from
Feb 10, 2025

Conversation

janbaykara
Copy link
Member

@janbaykara janbaykara commented Feb 9, 2025

Description

From a user perspective, this PR makes configuring choropleths more self-evident, and less janky/buggy/gotcha'd.

The three standard modes (count, field and formula) are complemented by a fourth: "advanced", which opens up a popup for you to edit the more complicated and fanciful options we've added recently.

Feb-09-2025 14-53-46

Backend

In the backend, there's been a bit of refactoring. The map and explorer now always uses the same statistical maths and querying logic:

  • We've removed the old choroplethDataForSource and genericDataSummaryFromSourceAboutArea resolvers and moved everything to the new statistics method (resolved by statistics and statisticsForChoropleth).
  • There's a new filter_generic_data_using_gss_code function.
  • Abstracted calculated values (like first, second) into add_computed_columns.

How Can It Be Tested?

  • Firstly, existing map reports in your localhost should upgrade without a hitch via the migration code: choropleths should generally continue to work just as they were previously configured.
  • Load up a list of members: count mode should work without the advanced editor.
  • Load up a list of area stats: fields should be self-evidently configurable now without the advanced editor.
  • Similarly, formulas.
  • Open up the advanced editor and do something fancy. For example, load the GE2024 ward results, select Nominal data type, then shade by \Winner (2024)`(including the backticks), and enableAre these electoral parties?`: you should see a party winner map!
  • Explorer displays should work as confusingly as they did before — reorganising them so they reliably show what we want is a bit out of the scope of this PR

@commonknowledge-bot commonknowledge-bot deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-database PR #213 February 9, 2025 14:45 — with Render Active
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-backend PR #213 February 9, 2025 14:46 — with Render Destroyed
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-backend PR #213 February 9, 2025 14:46 — with Render Destroyed
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-frontend PR #213 February 9, 2025 14:46 — with Render Destroyed
@commonknowledge-bot commonknowledge-bot deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-worker PR #213 February 9, 2025 14:46 — with Render Active
Copy link

linear bot commented Feb 9, 2025

@commonknowledge-bot commonknowledge-bot deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-worker PR #213 February 9, 2025 15:38 — with Render Active
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-backend PR #213 February 9, 2025 17:52 — with Render Destroyed
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-worker PR #213 February 9, 2025 17:52 — with Render Destroyed
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-frontend PR #213 February 9, 2025 19:25 — with Render Destroyed
@commonknowledge-bot commonknowledge-bot temporarily deployed to feature/map-1005-migrate-over-to-new-statistics-api - meep-intelligence-hub-frontend PR #213 February 9, 2025 19:26 — with Render Destroyed
@janbaykara janbaykara requested a review from joaquimds February 9, 2025 19:27
@janbaykara janbaykara changed the title Migrate everything to the new statistics API Migrate everything to the new statistics API / choropleth UX smoothing Feb 9, 2025
Copy link

@joaquimds joaquimds left a comment

Choose a reason for hiding this comment

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

lgtm, thanks Jan 👍

@joaquimds joaquimds merged commit f33b000 into main Feb 10, 2025
4 checks passed
@joaquimds joaquimds deleted the feature/map-1005-migrate-over-to-new-statistics-api branch February 10, 2025 11:38
Copy link

sentry-io bot commented Feb 10, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ GraphQLError: "['count'] not in index" /graphql View Issue

Did you find this useful? React with a 👍 or 👎

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.

3 participants