diff --git a/every_election/apps/api/serializers.py b/every_election/apps/api/serializers.py index 2713e8032..f5024e521 100644 --- a/every_election/apps/api/serializers.py +++ b/every_election/apps/api/serializers.py @@ -5,11 +5,13 @@ ModerationStatuses, ) from organisations.models import ( + DivisionGeography, Organisation, OrganisationDivision, OrganisationDivisionSet, ) from rest_framework import serializers +from rest_framework.reverse import reverse from rest_framework_gis.serializers import ( GeoFeatureModelSerializer, GeometrySerializerMethodField, @@ -64,6 +66,51 @@ class Meta: fields = org_fields +class OrganisationCurrentDivisionSetSerializer(serializers.ModelSerializer): + division_count = serializers.SerializerMethodField() + + def get_division_count(self, obj): + return obj.divisions.count() + + class Meta: + model = OrganisationDivisionSet + fields = ( + "start_date", + "end_date", + "short_title", + "legislation_url", + "consultation_url", + "notes", + "division_count", + ) + + +class OrganisationDetailSerializer(OrganisationSerializer): + """Extends OrganisationSerializer with divisionset fields. Use prefetch_related("divisionset") + on the queryset to avoid N+1 queries when listing multiple organisations.""" + + current_divisionset = serializers.SerializerMethodField() + divisionsets_url = serializers.SerializerMethodField() + + def get_current_divisionset(self, obj): + # Iterates the prefetch cache when available instead of issuing a new query. + active = [ds for ds in obj.divisionset.all() if ds.end_date is None] + if len(active) != 1: + return None + return OrganisationCurrentDivisionSetSerializer(active[0]).data + + def get_divisionsets_url(self, obj): + request = self.context.get("request") + url = reverse("api:division-list") + full_url = f"{url}?org_slug={obj.slug}" + if request: + return request.build_absolute_uri(full_url) + return full_url + + class Meta(OrganisationSerializer.Meta): + fields = org_fields + ("current_divisionset", "divisionsets_url") + + class OrganisationGeoSerializer(GeoFeatureModelSerializer): geography_model = GeometrySerializerMethodField() url = OrganisationHyperlinkedIdentityField( @@ -93,6 +140,8 @@ class Meta: class OrganisationDivisionSerializer(serializers.ModelSerializer): + """Base division serializer — used when a division is nested inside an election.""" + divisionset = OrganisationDivisionSetSerializer() class Meta: @@ -112,6 +161,133 @@ class Meta: ) +class DivisionOrganisationSerializer(serializers.ModelSerializer): + class Meta: + model = Organisation + fields = ( + "official_identifier", + "organisation_type", + "slug", + ) + + +class FlatDivisionSerializer(OrganisationDivisionSerializer): + """Division serializer for the flat /api/divisions/ endpoint — includes parent organisation.""" + + organisation = DivisionOrganisationSerializer(source="divisionset.organisation") + + class Meta(OrganisationDivisionSerializer.Meta): + fields = ( + "divisionset", + "organisation", + "name", + "official_identifier", + "slug", + "division_type", + "division_subtype", + "division_election_sub_type", + "seats_total", + "territory_code", + "created", + "modified", + ) + + +class DivisionGeoSerializer(GeoFeatureModelSerializer): + geography_model = GeometrySerializerMethodField() + + def get_geography_model(self, obj): + try: + return obj.geography.geography + except DivisionGeography.DoesNotExist: + return None + + class Meta: + model = OrganisationDivision + geo_field = "geography_model" + fields = ( + "name", + "official_identifier", + "slug", + "division_type", + "division_subtype", + "seats_total", + "territory_code", + ) + + +class DivisionForDivisionSetSerializer(serializers.ModelSerializer): + geo_url = serializers.SerializerMethodField() + geography_url = serializers.SerializerMethodField() + + def get_geo_url(self, obj): + request = self.context.get("request") + url = reverse("api:division-geo", args=[obj.pk]) + if request: + return request.build_absolute_uri(url) + return url + + def get_geography_url(self, obj): + return obj.format_geography_link() + + class Meta: + model = OrganisationDivision + fields = ( + "name", + "official_identifier", + "slug", + "division_type", + "division_subtype", + "division_election_sub_type", + "seats_total", + "territory_code", + "geo_url", + "geography_url", + ) + + +class DivisionSetOrganisationSerializer(serializers.ModelSerializer): + url = OrganisationHyperlinkedIdentityField( + view_name="api:organisation-detail", read_only=True + ) + + class Meta: + model = Organisation + fields = ( + "url", + "official_identifier", + "organisation_type", + "slug", + ) + + +class DivisionSetSerializer(serializers.ModelSerializer): + url = serializers.HyperlinkedIdentityField( + view_name="api:divisionset-detail", read_only=True + ) + organisation = DivisionSetOrganisationSerializer() + divisions = serializers.SerializerMethodField() + + def get_divisions(self, obj): + return DivisionForDivisionSetSerializer( + obj.divisions.all(), many=True, context=self.context + ).data + + class Meta: + model = OrganisationDivisionSet + fields = ( + "url", + "organisation", + "start_date", + "end_date", + "short_title", + "legislation_url", + "consultation_url", + "notes", + "divisions", + ) + + class ElectionTypeSerializer(serializers.HyperlinkedModelSerializer): class Meta: model = ElectionType diff --git a/every_election/apps/api/tests/test_api_divisions_endpoint.py b/every_election/apps/api/tests/test_api_divisions_endpoint.py new file mode 100644 index 000000000..866d84d19 --- /dev/null +++ b/every_election/apps/api/tests/test_api_divisions_endpoint.py @@ -0,0 +1,113 @@ +from datetime import date + +from django.test import override_settings +from organisations.tests.factories import ( + OrganisationDivisionFactory, + OrganisationDivisionSetFactory, + OrganisationFactory, +) +from rest_framework.test import APITestCase + + +class TestDivisionsEndpoint(APITestCase): + def setUp(self): + super().setUp() + self.local_org = OrganisationFactory( + official_identifier="STR", + organisation_type="local-authority", + start_date=date(2019, 4, 1), + end_date=None, + ) + self.active_ds = OrganisationDivisionSetFactory( + organisation=self.local_org, + start_date=date(2019, 5, 2), + end_date=None, + short_title="Stroud 2019 Review", + ) + self.historical_ds = OrganisationDivisionSetFactory( + organisation=self.local_org, + start_date=date(2010, 1, 1), + end_date=date(2019, 5, 1), + short_title="Stroud 2010 Review", + ) + self.division_active = OrganisationDivisionFactory( + divisionset=self.active_ds, + name="Amberley", + ) + self.division_historical = OrganisationDivisionFactory( + divisionset=self.historical_ds, + name="Old Ward", + ) + + self.other_org = OrganisationFactory( + official_identifier="PCC1", + organisation_type="combined-authority", + start_date=date(2017, 1, 1), + end_date=None, + ) + self.other_ds = OrganisationDivisionSetFactory( + organisation=self.other_org, + start_date=date(2017, 1, 1), + end_date=None, + ) + self.other_division = OrganisationDivisionFactory( + divisionset=self.other_ds, + name="Other Division", + ) + + def test_list_returns_200_with_pagination(self): + resp = self.client.get("/api/divisions/") + self.assertEqual(200, resp.status_code) + data = resp.json() + self.assertIn("count", data) + self.assertIn("results", data) + + def test_list_result_shape(self): + resp = self.client.get("/api/divisions/") + data = resp.json() + result = data["results"][0] + self.assertIn("divisionset", result) + self.assertIn("organisation", result) + self.assertIn("name", result) + + def test_filter_by_org_type(self): + resp = self.client.get("/api/divisions/?org_type=local-authority") + data = resp.json() + org_types = { + r["organisation"]["organisation_type"] for r in data["results"] + } + self.assertEqual({"local-authority"}, org_types) + + def test_filter_by_org_slug(self): + resp = self.client.get( + f"/api/divisions/?org_slug={self.local_org.slug}" + ) + data = resp.json() + slugs = {r["organisation"]["slug"] for r in data["results"]} + self.assertEqual({self.local_org.slug}, slugs) + # should include both active and historical divisionset divisions + self.assertEqual(2, data["count"]) + + def test_filter_current_excludes_historical(self): + resp = self.client.get("/api/divisions/?current=true") + data = resp.json() + for result in data["results"]: + self.assertIsNone(result["divisionset"]["end_date"]) + + def test_detail_endpoint(self): + resp = self.client.get(f"/api/divisions/{self.division_active.pk}/") + self.assertEqual(200, resp.status_code) + data = resp.json() + self.assertEqual("Amberley", data["name"]) + self.assertIn("divisionset", data) + self.assertIn("organisation", data) + self.assertEqual("STR", data["organisation"]["official_identifier"]) + + @override_settings(DEBUG=False) + def test_query_count_select_related(self): + for i in range(10): + OrganisationDivisionFactory(divisionset=self.active_ds) + + # With select_related, one query for count + one for data (no per-row extras) + with self.assertNumQueries(2): + self.client.get("/api/divisions/?limit=10") diff --git a/every_election/apps/api/tests/test_api_divisionsets_endpoint.py b/every_election/apps/api/tests/test_api_divisionsets_endpoint.py new file mode 100644 index 000000000..3bd544adb --- /dev/null +++ b/every_election/apps/api/tests/test_api_divisionsets_endpoint.py @@ -0,0 +1,142 @@ +from datetime import date, timedelta + +from organisations.tests.factories import ( + OrganisationDivisionFactory, + OrganisationDivisionSetFactory, + OrganisationFactory, +) +from rest_framework.test import APITestCase + + +class TestDivisionSetsEndpoint(APITestCase): + def setUp(self): + super().setUp() + self.local_org = OrganisationFactory( + official_identifier="STR", + organisation_type="local-authority", + start_date=date(2019, 4, 1), + end_date=None, + ) + self.active_ds = OrganisationDivisionSetFactory( + organisation=self.local_org, + start_date=date(2019, 5, 2), + end_date=None, + short_title="Stroud 2019 Review", + ) + self.historical_ds = OrganisationDivisionSetFactory( + organisation=self.local_org, + start_date=date(2010, 1, 1), + end_date=date(2019, 5, 1), + short_title="Stroud 2010 Review", + ) + self.div1 = OrganisationDivisionFactory( + divisionset=self.active_ds, + name="Amberley", + official_identifier="gss:E05013500", + ) + self.div2 = OrganisationDivisionFactory( + divisionset=self.active_ds, + name="Bisley", + ) + + self.other_org = OrganisationFactory( + official_identifier="PCC1", + organisation_type="combined-authority", + start_date=date(2017, 1, 1), + end_date=None, + ) + self.other_ds = OrganisationDivisionSetFactory( + organisation=self.other_org, + start_date=date(2017, 1, 1), + end_date=None, + ) + + def test_list_returns_200(self): + resp = self.client.get("/api/divisionsets/") + self.assertEqual(200, resp.status_code) + data = resp.json() + self.assertIn("count", data) + self.assertIn("results", data) + + def test_result_shape(self): + resp = self.client.get(f"/api/divisionsets/{self.active_ds.pk}/") + self.assertEqual(200, resp.status_code) + data = resp.json() + self.assertIn("url", data) + self.assertIn("organisation", data) + self.assertIn("start_date", data) + self.assertIn("end_date", data) + self.assertIn("short_title", data) + self.assertIn("divisions", data) + + def test_organisation_fields_on_divisionset(self): + resp = self.client.get(f"/api/divisionsets/{self.active_ds.pk}/") + data = resp.json() + org = data["organisation"] + self.assertEqual("STR", org["official_identifier"]) + self.assertEqual("local-authority", org["organisation_type"]) + self.assertIn("url", org) + + def test_divisions_embedded_in_divisionset(self): + resp = self.client.get(f"/api/divisionsets/{self.active_ds.pk}/") + data = resp.json() + self.assertEqual(2, len(data["divisions"])) + division = next(d for d in data["divisions"] if d["name"] == "Amberley") + self.assertIn("geo_url", division) + self.assertIn("geography_url", division) + + def test_division_geo_url_points_to_divisions_endpoint(self): + resp = self.client.get(f"/api/divisionsets/{self.active_ds.pk}/") + data = resp.json() + division = next(d for d in data["divisions"] if d["name"] == "Amberley") + self.assertIn(f"/api/divisions/{self.div1.pk}/geo", division["geo_url"]) + + def test_division_geography_url_is_mapit_link_for_gss(self): + resp = self.client.get(f"/api/divisionsets/{self.active_ds.pk}/") + data = resp.json() + division = next(d for d in data["divisions"] if d["name"] == "Amberley") + self.assertIn("mapit.mysociety.org", division["geography_url"]) + + def test_filter_by_official_identifier(self): + resp = self.client.get("/api/divisionsets/?official_identifier=STR") + data = resp.json() + self.assertEqual(2, data["count"]) + for result in data["results"]: + self.assertEqual( + "STR", result["organisation"]["official_identifier"] + ) + + def test_filter_by_start_date(self): + resp = self.client.get("/api/divisionsets/?start_date=2019-05-02") + data = resp.json() + self.assertEqual(1, data["count"]) + self.assertEqual("2019-05-02", data["results"][0]["start_date"]) + + def test_filter_current_excludes_historical(self): + resp = self.client.get("/api/divisionsets/?current=true") + data = resp.json() + for result in data["results"]: + self.assertIsNone(result["end_date"]) + + def test_filter_current_includes_future_end_date(self): + future_org = OrganisationFactory( + official_identifier="FUT", + organisation_type="local-authority", + start_date=date(2020, 1, 1), + end_date=None, + ) + future_ds = OrganisationDivisionSetFactory( + organisation=future_org, + start_date=date(2020, 1, 1), + end_date=date.today() + timedelta(days=30), + ) + resp = self.client.get("/api/divisionsets/?current=true") + data = resp.json() + ids = [r["url"] for r in data["results"]] + self.assertTrue(any(str(future_ds.pk) in u for u in ids)) + + def test_detail_url_is_absolute(self): + resp = self.client.get(f"/api/divisionsets/{self.active_ds.pk}/") + data = resp.json() + self.assertTrue(data["url"].startswith("http")) + self.assertIn(str(self.active_ds.pk), data["url"]) diff --git a/every_election/apps/api/tests/test_api_organisation_endpoint.py b/every_election/apps/api/tests/test_api_organisation_endpoint.py index e10ee1b69..b4e452b0d 100644 --- a/every_election/apps/api/tests/test_api_organisation_endpoint.py +++ b/every_election/apps/api/tests/test_api_organisation_endpoint.py @@ -1,6 +1,8 @@ from datetime import date from organisations.tests.factories import ( + OrganisationDivisionFactory, + OrganisationDivisionSetFactory, OrganisationFactory, OrganisationGeographyFactory, ) @@ -88,3 +90,69 @@ def test_filter_orgs_type_id_not_found(self): data = resp.json() self.assertEqual(200, resp.status_code) self.assertEqual([], data["results"]) + + +class TestOrganisationDivisionSetFields(APITestCase): + def setUp(self): + super().setUp() + self.org = OrganisationFactory( + official_identifier="STR", + organisation_type="local-authority", + start_date=date(2019, 4, 1), + end_date=None, + ) + self.active_ds = OrganisationDivisionSetFactory( + organisation=self.org, + start_date=date(2019, 5, 2), + end_date=None, + short_title="Stroud 2019 Review", + ) + OrganisationDivisionFactory(divisionset=self.active_ds) + OrganisationDivisionFactory(divisionset=self.active_ds) + + self.org_url = "/api/organisations/local-authority/STR/2019-04-01.json" + + def test_response_includes_divisionset_keys(self): + resp = self.client.get(self.org_url) + self.assertEqual(200, resp.status_code) + data = resp.json() + self.assertIn("current_divisionset", data) + self.assertIn("divisionsets_url", data) + + def test_current_divisionset_non_null_for_active_org(self): + resp = self.client.get(self.org_url) + data = resp.json() + self.assertIsNotNone(data["current_divisionset"]) + self.assertEqual( + "Stroud 2019 Review", data["current_divisionset"]["short_title"] + ) + self.assertIsNone(data["current_divisionset"]["end_date"]) + + def test_current_divisionset_null_when_all_historical(self): + org2 = OrganisationFactory( + official_identifier="OLD", + organisation_type="local-authority", + start_date=date(2010, 1, 1), + end_date=None, + ) + OrganisationDivisionSetFactory( + organisation=org2, + start_date=date(2010, 1, 1), + end_date=date(2019, 1, 1), + ) + resp = self.client.get( + "/api/organisations/local-authority/OLD/2010-01-01.json" + ) + data = resp.json() + self.assertIsNone(data["current_divisionset"]) + + def test_current_divisionset_division_count(self): + resp = self.client.get(self.org_url) + data = resp.json() + self.assertEqual(2, data["current_divisionset"]["division_count"]) + + def test_divisionsets_url_contains_org_slug(self): + resp = self.client.get(self.org_url) + data = resp.json() + self.assertIn(f"?org_slug={self.org.slug}", data["divisionsets_url"]) + self.assertTrue(data["divisionsets_url"].startswith("http")) diff --git a/every_election/apps/api/urls.py b/every_election/apps/api/urls.py index b9e0de994..9826b7740 100644 --- a/every_election/apps/api/urls.py +++ b/every_election/apps/api/urls.py @@ -5,6 +5,8 @@ ElectionSubTypeViewSet, ElectionTypeViewSet, ElectionViewSet, + OrganisationDivisionSetViewSet, + OrganisationDivisionViewSet, OrganisationViewSet, ) @@ -26,6 +28,8 @@ def get_lookup_regex(self, viewset, lookup_prefix=""): router.register(r"election_types", ElectionTypeViewSet) router.register(r"election_subtypes", ElectionSubTypeViewSet) router.register(r"organisations", OrganisationViewSet) +router.register(r"divisions", OrganisationDivisionViewSet, basename="division") +router.register(r"divisionsets", OrganisationDivisionSetViewSet, basename="divisionset") routes = router.get_urls() diff --git a/every_election/apps/api/views.py b/every_election/apps/api/views.py index 3ae5e47b2..3f6eff788 100644 --- a/every_election/apps/api/views.py +++ b/every_election/apps/api/views.py @@ -1,9 +1,9 @@ from collections import OrderedDict -from datetime import datetime +from datetime import date, datetime from api import filters from django.conf import settings -from django.db.models import Prefetch +from django.db.models import Prefetch, Q from django.http import Http404 from elections.models import ( Election, @@ -12,7 +12,7 @@ ModerationStatuses, ) from elections.query_helpers import PostcodeError -from organisations.models import Organisation, OrganisationDivision +from organisations.models import Organisation, OrganisationDivision, OrganisationDivisionSet from rest_framework import viewsets from rest_framework.decorators import action from rest_framework.exceptions import APIException @@ -20,10 +20,14 @@ from uk_election_ids.election_ids import validate from .serializers import ( + DivisionGeoSerializer, + DivisionSetSerializer, ElectionGeoSerializer, ElectionSerializer, ElectionSubTypeSerializer, ElectionTypeSerializer, + FlatDivisionSerializer, + OrganisationDetailSerializer, OrganisationDivisionSerializer, OrganisationGeoSerializer, OrganisationSerializer, @@ -70,6 +74,7 @@ def get_queryset(self): "elected_role", "division", "division__divisionset", + "division__divisionset__organisation", "group", "replaces", "metadata", @@ -197,7 +202,9 @@ class OrganisationViewSet(viewsets.ReadOnlyModelViewSet): def get_object(self, **kwargs): kwargs["date"] = datetime.strptime(kwargs["date"], "%Y-%m-%d").date() try: - return Organisation.objects.all().get_by_date(**kwargs) + return Organisation.objects.prefetch_related("divisionset").get_by_date( + **kwargs + ) except Organisation.DoesNotExist: raise Http404() @@ -247,19 +254,19 @@ def elections(self, request, **kwargs): def retrieve(self, request, **kwargs): kwargs.pop("format", None) org = self.get_object(**kwargs) - serializer = OrganisationSerializer( + serializer = OrganisationDetailSerializer( org, read_only=True, context={"request": request} ) return Response(serializer.data) def filter(self, request, **kwargs): kwargs.pop("format", None) - orgs = Organisation.objects.all().filter(**kwargs) + orgs = Organisation.objects.prefetch_related("divisionset").filter(**kwargs) page = self.paginate_queryset(orgs) if page is not None: return self.get_paginated_response( - OrganisationSerializer( + OrganisationDetailSerializer( page, many=True, read_only=True, @@ -268,13 +275,65 @@ def filter(self, request, **kwargs): ) return Response( - OrganisationSerializer( + OrganisationDetailSerializer( orgs, many=True, read_only=True, context={"request": request} ).data ) class OrganisationDivisionViewSet(viewsets.ReadOnlyModelViewSet): - queryset = OrganisationDivision.objects.all() - serializer_class = OrganisationDivisionSerializer + serializer_class = FlatDivisionSerializer filterset_fields = ["modified"] + + def get_queryset(self): + qs = OrganisationDivision.objects.select_related( + "divisionset", + "divisionset__organisation", + ).order_by("divisionset__organisation__organisation_type", "name") + + org_type = self.request.query_params.get("org_type") + if org_type: + qs = qs.filter(divisionset__organisation__organisation_type=org_type) + + org_slug = self.request.query_params.get("org_slug") + if org_slug: + qs = qs.filter(divisionset__organisation__slug=org_slug) + + current = self.request.query_params.get("current") + if current == "true": + qs = qs.filter(divisionset__end_date__isnull=True) + + return qs + + @action(detail=True, url_path="geo") + def geo(self, request, pk=None): + division = self.get_object() + return Response( + DivisionGeoSerializer(division, context={"request": request}).data + ) + + +class OrganisationDivisionSetViewSet(viewsets.ReadOnlyModelViewSet): + serializer_class = DivisionSetSerializer + + def get_queryset(self): + qs = OrganisationDivisionSet.objects.select_related( + "organisation", + ).prefetch_related( + "divisions", + ).order_by("organisation__organisation_type", "-start_date") + + official_identifier = self.request.query_params.get("official_identifier") + if official_identifier: + qs = qs.filter(organisation__official_identifier=official_identifier) + + start_date = self.request.query_params.get("start_date") + if start_date: + qs = qs.filter(start_date=start_date) + + current = self.request.query_params.get("current") + if current == "true": + today = date.today() + qs = qs.filter(Q(end_date__isnull=True) | Q(end_date__gte=today)) + + return qs