diff --git a/bookwyrm/migrations/0182_auto_20231023_0246.py b/bookwyrm/migrations/0182_auto_20231023_0246.py new file mode 100644 index 000000000..d3db4056b --- /dev/null +++ b/bookwyrm/migrations/0182_auto_20231023_0246.py @@ -0,0 +1,54 @@ +# Generated by Django 3.2.20 on 2023-10-23 02:46 + +import bookwyrm.models.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0181_merge_20230806_2302"), + ] + + operations = [ + migrations.AddField( + model_name="book", + name="first_published_date_precision", + field=models.CharField( + blank=True, + choices=[ + ("DAY", "Day seal"), + ("MONTH", "Month seal"), + ("YEAR", "Year seal"), + ], + editable=False, + max_length=10, + null=True, + ), + ), + migrations.AddField( + model_name="book", + name="published_date_precision", + field=models.CharField( + blank=True, + choices=[ + ("DAY", "Day seal"), + ("MONTH", "Month seal"), + ("YEAR", "Year seal"), + ], + editable=False, + max_length=10, + null=True, + ), + ), + migrations.AlterField( + model_name="book", + name="first_published_date", + field=bookwyrm.models.fields.PartialDateField(blank=True, null=True), + ), + migrations.AlterField( + model_name="book", + name="published_date", + field=bookwyrm.models.fields.PartialDateField(blank=True, null=True), + ), + ] diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 9e05c03af..f0a524774 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -135,8 +135,8 @@ class Book(BookDataModel): preview_image = models.ImageField( upload_to="previews/covers/", blank=True, null=True ) - first_published_date = fields.DateTimeField(blank=True, null=True) - published_date = fields.DateTimeField(blank=True, null=True) + first_published_date = fields.PartialDateField(blank=True, null=True) + published_date = fields.PartialDateField(blank=True, null=True) objects = InheritanceManager() field_tracker = FieldTracker(fields=["authors", "title", "subtitle", "cover"]) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 28effaf9b..9c8793649 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -20,6 +20,11 @@ from markdown import markdown from bookwyrm import activitypub from bookwyrm.connectors import get_image from bookwyrm.utils.sanitizer import clean +from bookwyrm.utils.sealed_date import ( + SealedDate, + SealedDateField, + from_partial_isoformat, +) from bookwyrm.settings import MEDIA_FULL_URL @@ -537,7 +542,6 @@ class DateTimeField(ActivitypubFieldMixin, models.DateTimeField): def field_from_activity(self, value, allow_external_connections=True): missing_fields = datetime(1970, 1, 1) # "2022-10" => "2022-10-01" try: - # TODO(dato): investigate `ignoretz=True` wrt bookwyrm#3028. date_value = dateutil.parser.parse(value, default=missing_fields) try: return timezone.make_aware(date_value) @@ -547,6 +551,34 @@ class DateTimeField(ActivitypubFieldMixin, models.DateTimeField): return None +class PartialDateField(ActivitypubFieldMixin, SealedDateField): + """activitypub-aware partial date field""" + + def field_to_activity(self, value) -> str: + return value.partial_isoformat() if value else None + + def field_from_activity(self, value, allow_external_connections=True): + try: + return from_partial_isoformat(value) + except ValueError: + pass + + # fallback to full ISO-8601 parsing + try: + parsed = dateutil.parser.isoparse(value) + except (ValueError, ParserError): + return None + + # FIXME #1: add timezone if missing (SealedDate only accepts tz-aware). + # + # FIXME #2: decide whether to fix timestamps like "2023-09-30T21:00:00-03": + # clearly Oct 1st, not Sep 30th (an unwanted side-effect of USE_TZ). It's + # basically the remnants of #3028; there is a data migration pending (see …) + # but over the wire we might get these for an indeterminate amount of time. + + return SealedDate.from_datetime(parsed) + + class HtmlField(ActivitypubFieldMixin, models.TextField): """a text field for storing html""" diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 553a533d5..1f4a18aef 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -2,10 +2,12 @@ from io import BytesIO from collections import namedtuple from dataclasses import dataclass +import datetime import json import pathlib import re from typing import List +from unittest import expectedFailure from unittest.mock import patch from PIL import Image @@ -23,6 +25,7 @@ from bookwyrm.models import fields, User, Status, Edition from bookwyrm.models.base_model import BookWyrmModel from bookwyrm.models.activitypub_mixin import ActivitypubMixin from bookwyrm.settings import DOMAIN +from bookwyrm.utils import sealed_date # pylint: disable=too-many-public-methods @patch("bookwyrm.suggested_users.rerank_suggestions_task.delay") @@ -594,6 +597,37 @@ class ModelFields(TestCase): self.assertEqual(instance.field_from_activity(now.isoformat()), now) self.assertEqual(instance.field_from_activity("bip"), None) + def test_partial_date_legacy_formats(self, *_): + """test support for full isoformat in partial dates""" + instance = fields.PartialDateField() + expected = datetime.date(2023, 10, 20) + test_cases = [ + # XXX: must fix before merging. + # ("no_tz", "2023-10-20T00:00:00"), + # ("no_tz_eod", "2023-10-20T23:59:59.999999"), + ("utc_offset_midday", "2023-10-20T12:00:00+0000"), + ("utc_offset_midnight", "2023-10-20T00:00:00+00"), + ("eastern_tz_parsed", "2023-10-20T15:20:30+04:30"), + ("western_tz_midnight", "2023-10-20:00:00-03"), + ] + for desc, value in test_cases: + with self.subTest(desc): + parsed = instance.field_from_activity(value) + self.assertIsNotNone(parsed) + self.assertEqual(expected, parsed.date()) + self.assertTrue(parsed.has_day) + self.assertTrue(parsed.has_month) + + @expectedFailure + def test_partial_date_timezone_fix(self, *_): + """deserialization compensates for unwanted effects of USE_TZ""" + instance = fields.PartialDateField() + expected = datetime.date(2023, 10, 1) + parsed = instance.field_from_activity("2023-09-30T21:00:00-03") + self.assertEqual(expected, parsed.date()) + self.assertTrue(parsed.has_day) + self.assertTrue(parsed.has_month) + def test_array_field(self, *_): """idk why it makes them strings but probably for a good reason""" instance = fields.ArrayField(fields.IntegerField) diff --git a/bookwyrm/tests/test_sealed_date.py b/bookwyrm/tests/test_sealed_date.py index cda1ae0fc..4e87a26d0 100644 --- a/bookwyrm/tests/test_sealed_date.py +++ b/bookwyrm/tests/test_sealed_date.py @@ -35,6 +35,10 @@ class SealedDateTest(unittest.TestCase): self.assertFalse(sealed.has_day) self.assertFalse(sealed.has_month) + def test_no_naive_datetime(self): + with self.assertRaises(ValueError): + sealed_date.SealedDate.from_datetime(datetime.datetime(2000, 1, 1)) + def test_parse_year_seal(self): parsed = sealed_date.from_partial_isoformat("1995") expected = datetime.date(1995, 1, 1) diff --git a/bookwyrm/tests/views/books/test_edit_book.py b/bookwyrm/tests/views/books/test_edit_book.py index 68553e09e..49e8c7cdb 100644 --- a/bookwyrm/tests/views/books/test_edit_book.py +++ b/bookwyrm/tests/views/books/test_edit_book.py @@ -1,5 +1,4 @@ """ test for app action functionality """ -from unittest import expectedFailure from unittest.mock import patch import responses from responses import matchers @@ -211,7 +210,6 @@ class EditBookViews(TestCase): book = models.Edition.objects.get(title="New Title") self.assertEqual(book.parent_work.title, "New Title") - @expectedFailure # bookwyrm#3028 def test_published_date_timezone(self): """user timezone does not affect publication year""" # https://github.com/bookwyrm-social/bookwyrm/issues/3028 @@ -234,6 +232,75 @@ class EditBookViews(TestCase): book = models.Edition.objects.get(title="January 1st test") self.assertEqual(book.edition_info, "2020") + def test_partial_published_dates(self): + """create a book with partial publication dates, then update them""" + self.local_user.groups.add(self.group) + book_data = { + "title": "An Edition With Dates", + "parent_work": self.work.id, + "last_edited_by": self.local_user.id, + } + initial_pub_dates = { + # published_date: 2023-01-01 + "published_date_day": "1", + "published_date_month": "01", + "published_date_year": "2023", + # first_published_date: 1995 + "first_published_date_day": "", + "first_published_date_month": "", + "first_published_date_year": "1995", + } + updated_pub_dates = { + # published_date: full -> year-only + "published_date_day": "", + "published_date_month": "", + "published_date_year": "2023", + # first_published_date: add month + "first_published_date_day": "", + "first_published_date_month": "03", + "first_published_date_year": "1995", + } + + # create book + create_book = views.CreateBook.as_view() + request = self.factory.post("", book_data | initial_pub_dates) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + create_book(request) + + book = models.Edition.objects.get(title="An Edition With Dates") + + self.assertEqual("2023-01-01", book.published_date.partial_isoformat()) + self.assertEqual("1995", book.first_published_date.partial_isoformat()) + + self.assertTrue(book.published_date.has_day) + self.assertTrue(book.published_date.has_month) + + self.assertFalse(book.first_published_date.has_day) + self.assertFalse(book.first_published_date.has_month) + + # now edit publication dates + edit_book = views.ConfirmEditBook.as_view() + request = self.factory.post("", book_data | updated_pub_dates) + request.user = self.local_user + + with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): + result = edit_book(request, book.id) + + self.assertEqual(result.status_code, 302) + + book.refresh_from_db() + + self.assertEqual("2023", book.published_date.partial_isoformat()) + self.assertEqual("1995-03", book.first_published_date.partial_isoformat()) + + self.assertFalse(book.published_date.has_day) + self.assertFalse(book.published_date.has_month) + + self.assertFalse(book.first_published_date.has_day) + self.assertTrue(book.first_published_date.has_month) + def test_create_book_existing_work(self): """create an entirely new book and work""" view = views.ConfirmEditBook.as_view() diff --git a/bookwyrm/utils/sealed_date.py b/bookwyrm/utils/sealed_date.py index 6055b03cc..c7ad3b7f3 100644 --- a/bookwyrm/utils/sealed_date.py +++ b/bookwyrm/utils/sealed_date.py @@ -7,6 +7,7 @@ import re from typing import Any, Optional, Type, TypeVar, cast from django.core.exceptions import ValidationError +from django.db import models from django.forms import DateField from django.forms.widgets import SelectDateWidget from django.utils import timezone @@ -22,11 +23,12 @@ _westmost_tz = timezone.get_fixed_timezone(timedelta(hours=-12)) Sealed = TypeVar("Sealed", bound="SealedDate") # TODO: use Self in Python >= 3.11 -# TODO: migrate SealedDate to `date` +# TODO: migrate SealedDate: `datetime` => `date` +# TODO: migrate SealedDateField: `DateTimeField` => `DateField` class SealedDate(datetime): - """a date object sealed into a certain precision (day, month, year)""" + """a date object sealed into a certain precision (day, month or year)""" @property def has_day(self) -> bool: @@ -42,6 +44,8 @@ class SealedDate(datetime): @classmethod def from_datetime(cls: Type[Sealed], dt: datetime) -> Sealed: # pylint: disable=invalid-name + if timezone.is_naive(dt): + raise ValueError("naive datetime not accepted") return cls.combine(dt.date(), dt.time(), tzinfo=dt.tzinfo) @classmethod @@ -120,3 +124,86 @@ class SealedDateFormField(DateField): year, month, day = date.year, date.month, date.day return SealedDate.from_date_parts(year, month, day) + + +class SealedDateDescriptor: + + _SEAL_TYPES = { + YearSeal: "YEAR", + MonthSeal: "MONTH", + SealedDate: "DAY", + } + + _DATE_CLASSES = { + "YEAR": YearSeal, + "MONTH": MonthSeal, + } + + def __init__(self, field): + self.field = field + + @property + def precision_field(self): + """the name of the accompanying precision field""" + return self.make_precision_name(self.field.attname) + + @classmethod + def make_precision_name(cls, date_attr_name): + # used by SealedDateField to make the name from the outside. + # TODO: migrate to an attribute there? + return f"{date_attr_name}_precision" + + @property + def precision_choices(self): + return (("DAY", "Day seal"), ("MONTH", "Month seal"), ("YEAR", "Year seal")) + + def __get__(self, instance, cls=None): + if instance is None: + return self + + value = instance.__dict__.get(self.field.attname) + + if not value or isinstance(value, SealedDate): + return value + + # use precision field to construct SealedDate. + seal_type = getattr(instance, self.precision_field, None) + date_class = self._DATE_CLASSES.get(seal_type, SealedDate) + + return date_class.from_datetime(value) # FIXME: drop datetimes. + + def __set__(self, instance, value): + """assign value, with precision where available""" + try: + seal_type = self._SEAL_TYPES[value.__class__] + except KeyError: + value = self.field.to_python(value) + else: + setattr(instance, self.precision_field, seal_type) + + instance.__dict__[self.field.attname] = value + + +class SealedDateField(models.DateTimeField): # FIXME: use DateField. + + descriptor_class = SealedDateDescriptor + + def formfield(self, **kwargs): + kwargs.setdefault("form_class", SealedDateFormField) + return super().formfield(**kwargs) + + # pylint: disable-next=arguments-renamed + def contribute_to_class(self, model, our_name_in_model, **kwargs): + # Define precision field. + descriptor = self.descriptor_class(self) + precision = models.CharField( + null=True, + blank=True, + editable=False, + max_length=10, + choices=descriptor.precision_choices, + ) + precision_name = descriptor.make_precision_name(our_name_in_model) + + model.add_to_class(precision_name, precision) + return super().contribute_to_class(model, our_name_in_model, **kwargs)