diff --git a/ynr/apps/candidates/views/people.py b/ynr/apps/candidates/views/people.py index 89c522a9b2..cb94a11f36 100644 --- a/ynr/apps/candidates/views/people.py +++ b/ynr/apps/candidates/views/people.py @@ -126,12 +126,16 @@ def get_context_data(self, **kwargs): def get(self, request, *args, **kwargs): person_id = self.kwargs["person_id"] try: - self.person = Person.objects.prefetch_related( - "memberships__ballot", - "memberships__ballot__post", - "memberships__ballot__election", - "tmp_person_identifiers", - ).get(pk=person_id) + self.person = ( + Person.objects.with_biography_last_updated() + .prefetch_related( + "memberships__ballot", + "memberships__ballot__post", + "memberships__ballot__election", + "tmp_person_identifiers", + ) + .get(pk=person_id) + ) except Person.DoesNotExist: try: return self.get_person_redirect(person_id) diff --git a/ynr/apps/data_exports/csv_fields.py b/ynr/apps/data_exports/csv_fields.py index 0667bb1782..160c2b7c60 100644 --- a/ynr/apps/data_exports/csv_fields.py +++ b/ynr/apps/data_exports/csv_fields.py @@ -20,11 +20,19 @@ from django.core.files.storage import default_storage from django.db.models import BooleanField, CharField, Expression -from django.db.models.expressions import Case, Combinable, F, Value, When +from django.db.models.expressions import ( + Case, + Combinable, + F, + RawSQL, + Value, + When, +) from django.db.models.functions import Concat, Substr from django.template.defaultfilters import truncatechars from django.urls import reverse from django.utils.safestring import SafeString +from people.managers import BIOGRAPHY_LAST_UPDATED_SQL from ynr_refactoring.settings import PersonIdentifierFields @@ -321,6 +329,7 @@ def link_formatter(value, url): value_group="person", label="Favourite Biscuit", ) + csv_fields["statement_to_voters"] = CSVField( type="expr", value=F("person__biography"), @@ -331,7 +340,7 @@ def link_formatter(value, url): csv_fields["statement_last_updated"] = CSVField( type="expr", - value=F("person__biography_last_updated"), + value=RawSQL(BIOGRAPHY_LAST_UPDATED_SQL, []), value_group="person", label="Statement last updated", ) diff --git a/ynr/apps/people/forms/forms.py b/ynr/apps/people/forms/forms.py index 1d13b543fe..51047e6571 100644 --- a/ynr/apps/people/forms/forms.py +++ b/ynr/apps/people/forms/forms.py @@ -6,7 +6,6 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.core.validators import URLValidator, validate_email -from django.utils import timezone from django.utils.functional import cached_property from django.utils.safestring import mark_safe from official_documents.models import BallotSOPN @@ -316,7 +315,6 @@ class Meta: "summary", "national_identity", "name_search_vector", - "biography_last_updated", "death_date", ) @@ -419,14 +417,6 @@ def save(self, commit=True, user=None): user=user, ) - initial_biography = self.initial.get("biography", None) - biography_updated = ( - "biography" in self.changed_data - and initial_biography != self.cleaned_data["biography"] - ) - if biography_updated: - self.instance.biography_last_updated = timezone.now() - return super().save(commit) diff --git a/ynr/apps/people/managers.py b/ynr/apps/people/managers.py index c9612ec14d..f8871c81d7 100644 --- a/ynr/apps/people/managers.py +++ b/ynr/apps/people/managers.py @@ -4,6 +4,7 @@ from candidates.models import PersonRedirect from django.core.files import File from django.db import connection, models +from django.db.models.expressions import RawSQL from ynr_refactoring.settings import PersonIdentifierFields @@ -107,6 +108,34 @@ def editable_value_types(self): ON people_person FOR EACH ROW EXECUTE PROCEDURE people_person_search_trigger(); """ +BIOGRAPHY_LAST_UPDATED_SQL = """ +CASE +WHEN biography = '' THEN NULL +ELSE ( + WITH expanded AS ( + SELECT + (elem->>'timestamp')::timestamptz AS ts, + elem->'data'->>'biography' AS bio + FROM jsonb_array_elements(versions) AS elem + ), + ordered AS ( + SELECT + ts, + bio, + LAG(bio) OVER (ORDER BY ts) AS prev_bio + FROM expanded + ) + SELECT ts + FROM ordered + WHERE prev_bio IS DISTINCT FROM bio + AND bio IS NOT NULL + AND bio <> '' + ORDER BY ts DESC + LIMIT 1 +) +END +""" + class PersonQuerySet(models.query.QuerySet): def alive_now(self): @@ -134,3 +163,8 @@ def update_name_search(self): def update_name_search_trigger(self): self._run_sql(NAME_SEARCH_TRIGGER_SQL) + + def with_biography_last_updated(self): + return self.annotate( + biography_last_updated=RawSQL(BIOGRAPHY_LAST_UPDATED_SQL, []) + ) diff --git a/ynr/apps/people/merging.py b/ynr/apps/people/merging.py index 42d0b22ba3..3b9fbbbaac 100644 --- a/ynr/apps/people/merging.py +++ b/ynr/apps/people/merging.py @@ -72,7 +72,6 @@ class PersonMerger: ("birth_date", "merge_person_attrs"), ("death_date", "merge_person_attrs"), ("biography", "merge_person_attrs"), - ("biography_last_updated", "merge_person_attrs"), ("other_names", "merge_name_and_other_names"), ("family_name", "merge_person_attrs"), ("national_identity", "merge_person_attrs"), diff --git a/ynr/apps/people/migrations/0048_remove_person_biography_last_updated.py b/ynr/apps/people/migrations/0048_remove_person_biography_last_updated.py new file mode 100644 index 0000000000..d872b253c9 --- /dev/null +++ b/ynr/apps/people/migrations/0048_remove_person_biography_last_updated.py @@ -0,0 +1,19 @@ +# Generated by Django 5.2.8 on 2025-11-26 13:00 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ( + "people", + "0047_alter_personimage_options_personimage_created_and_more", + ), + ] + + operations = [ + migrations.RemoveField( + model_name="person", + name="biography_last_updated", + ), + ] diff --git a/ynr/apps/people/models.py b/ynr/apps/people/models.py index beec087d43..2fe07c0e65 100644 --- a/ynr/apps/people/models.py +++ b/ynr/apps/people/models.py @@ -274,9 +274,6 @@ class Person(TimeStampedModel, models.Model): blank=True, help_text="An extended account of a person's life", ) - biography_last_updated = models.DateTimeField( - "biography last updated", null=True, blank=True - ) national_identity = models.CharField( "national identity", max_length=128, diff --git a/ynr/apps/people/tests/test_person_update.py b/ynr/apps/people/tests/test_person_update.py index e600b47587..742ee28abf 100644 --- a/ynr/apps/people/tests/test_person_update.py +++ b/ynr/apps/people/tests/test_person_update.py @@ -4,7 +4,7 @@ from django.urls import reverse from django.utils import timezone from freezegun import freeze_time -from people.models import EditLimitationStatuses +from people.models import EditLimitationStatuses, Person from people.tests.test_person_view import PersonViewSharedTestsMixin from webtest import Text @@ -458,15 +458,17 @@ def test_add_candidacy_does_not_update_biography_timestamp(self): form["source"] = "Mumsnet" form.submit() - self.person.refresh_from_db() + person = Person.objects.with_biography_last_updated().get( + pk=self.person.pk + ) - self.assertEqual(len(self.person.versions), 1) + self.assertEqual(len(person.versions), 1) person_response_one = self.app.get( - "/person/{}/".format(self.person.pk), user=self.user + "/person/{}/".format(person.pk), user=self.user ) biography_update_timestamp = ( - self.person.biography_last_updated.astimezone() + person.biography_last_updated.astimezone() ) # format into a string to compare with the response biography_update_timestamp = biography_update_timestamp.strftime( @@ -485,7 +487,7 @@ def test_add_candidacy_does_not_update_biography_timestamp(self): # the biography timestamp has not changed with freeze_time(later_timestamp): candidacy_update_response = self.app.get( - "/person/{}/update".format(self.person.pk), user=self.user + "/person/{}/update".format(person.pk), user=self.user ) form = candidacy_update_response.forms["person-details"] form["memberships-0-party_identifier_0"].select( @@ -501,25 +503,27 @@ def test_add_candidacy_does_not_update_biography_timestamp(self): form["source"] = "http://example.com" form.submit() - self.person.refresh_from_db() + person = Person.objects.with_biography_last_updated().get( + pk=self.person.pk + ) - candidacy = self.person.memberships.first() + candidacy = person.memberships.first() self.assertEqual(candidacy.party, self.green_party) self.assertEqual(candidacy.party_name, self.green_party.name) candidacy_update_timestamp = ( - self.person.biography_last_updated.astimezone() + person.biography_last_updated.astimezone() ) candidacy_update_timestamp = candidacy_update_timestamp.strftime( "%-d %B %Y %H:%M" ) person_response_one = self.app.get( - "/person/{}/".format(self.person.pk), user=self.user + "/person/{}/".format(person.pk), user=self.user ) - self.assertEqual(len(self.person.versions), 2) - self.assertNotEqual(self.person.biography_last_updated, None) + self.assertEqual(len(person.versions), 2) + self.assertNotEqual(person.biography_last_updated, None) # assert that timestamp in the first update and second update are the same # because the biography should not have been updated in the second update self.assertEqual(