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

Timestamp for PersonImage #2413

Merged
merged 1 commit into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ynr/apps/candidates/templates/candidates/_photo-credit.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
{% with user_comment=image.user_notes %}

{% if contributor %}
This photo was uploaded by the user ‘{{ contributor }}’.
This photo was uploaded by the user ‘{{ contributor }}’ on {{ image.modified }}.
{% else%}
This photo was uploaded on {{ image.modified }}.
{% endif %}
{% if user_reason and user_reason != 'other' %}
Their justification for its use on the site was:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Generated by Django 4.2.11 on 2024-07-02 13:44

import django.utils.timezone
import django_extensions.db.fields
from django.db import migrations


def get_timestamp_from_queued_image(apps, schema_editor):
"""Set the created and modified timestamps on the image
to the timestamp of the first and last approved queued image"""

PersonImage = apps.get_model("people", "PersonImage")
for personimage in PersonImage.objects.all():
person = personimage.person
queued_images = person.queuedimage_set.all()
if not queued_images:
continue
modified = queued_images.filter(decision="approved").last().updated
created = queued_images.filter(decision="approved").last().created
if modified:
personimage.modified = modified
personimage.created = created
personimage.save()


class Migration(migrations.Migration):
dependencies = [
("people", "0046_alter_personidentifier_unique_together"),
]

operations = [
migrations.AlterModelOptions(
name="personimage",
options={"get_latest_by": "modified"},
),
migrations.AddField(
model_name="personimage",
name="created",
field=django_extensions.db.fields.CreationDateTimeField(
auto_now_add=True,
default=django.utils.timezone.now,
verbose_name="created",
),
preserve_default=False,
),
migrations.AddField(
model_name="personimage",
name="modified",
field=django_extensions.db.fields.ModificationDateTimeField(
auto_now=True, verbose_name="modified"
),
),
(
migrations.RunPython(
get_timestamp_from_queued_image, migrations.RunPython.noop
)
),
]
2 changes: 1 addition & 1 deletion ynr/apps/people/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class EditLimitationStatuses(Enum):
EDITS_PREVENTED = "Edits prevented"


class PersonImage(models.Model):
class PersonImage(TimeStampedModel):
"""
Images of people, uploaded by users of the site. It's important we keep
track of the copyright the uploading user asserts over the image, and any
Expand Down
37 changes: 37 additions & 0 deletions ynr/apps/people/tests/test_person_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from candidates.tests.helpers import TmpMediaRootMixin
from candidates.tests.uk_examples import UK2015ExamplesMixin
from django.contrib.auth import get_user_model
from django.utils import timezone
from django_webtest import WebTest
from moderation_queue.models import QueuedImage
from moderation_queue.tests.paths import EXAMPLE_IMAGE_FILENAME
Expand Down Expand Up @@ -47,6 +48,42 @@ def test_get_display_image_url(self):
person = Person.objects.get()
self.assertEqual(person.get_display_image_url(), url)

def test_all_person_images_have_a_timestamp(self):
"""ensure that all person images have a timestamp"""
person = PersonFactory(name=faker_factory.name())
pi = PersonImage.objects.create_from_file(
filename=EXAMPLE_IMAGE_FILENAME,
new_filename="images/jowell-pilot.jpg",
defaults={
"person": person,
"source": "Taken from Wikipedia",
"copyright": "example-license",
"user_notes": "A photo of Tessa Jowell",
},
)
self.assertIsNotNone(pi.created)
self.assertIsNotNone(pi.modified)

def test_person_image_modified_timestamp(self):
"""visit the person-view.html template and check the response for the modified timestamp"""
person = PersonFactory(name=faker_factory.name())
pi = PersonImage.objects.create_from_file(
filename=EXAMPLE_IMAGE_FILENAME,
new_filename="images/jowell-pilot.jpg",
defaults={
"person": person,
"source": "Taken from Wikipedia",
"copyright": "example-license",
"user_notes": "A photo of Tessa Jowell",
},
)
modified = timezone.localtime(pi.modified)
response = self.app.get(f"/person/{person.id}", user=self.user)
self.assertIn(
Copy link
Contributor Author

@VirginiaDooley VirginiaDooley Jul 2, 2024

Choose a reason for hiding this comment

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

BST problems? I tried this again with freeze_time and also changed the migration update and created default to datetime.now(timezone.utc).isoformat() but got the same error.

Copy link
Member

Choose a reason for hiding this comment

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

Is this working now? CI seems ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I don't think so - it's a mismatch in the time.

Copy link
Member

Choose a reason for hiding this comment

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

I've pushed a commit that fixes this, but I'm a little confused as to why we're not getting the right timezone back from pi.modified. In my understanding, the model should store the timezone and return local time, but maybe that's not how it works.

f"This photo was uploaded on {modified.strftime('%-d %B %Y %H:%M')}.",
response.text,
)

def test_get_alive_now(self):
alive_person = PersonFactory(name=faker_factory.name())
PersonFactory(name=faker_factory.name(), death_date="2016-01-01")
Expand Down