From a27fcc4c697a1503ff761608048836a04f5d1caf Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Sat, 5 Sep 2026 10:00:45 -0600 Subject: [PATCH] Fix task edit crashing after due_date/due_time save (regression from reminders) The web edit/create/quick-add views and the sync endpoint assigned due_date/due_time/reminder_at straight from request data as raw strings. Django tolerates this at save() (types get coerced deep in the SQL layer), but the in-memory instance keeps the raw strings afterward. Task.reschedule_reminders(), added for per-task reminders and run from save() on that same instance, was the first code to actually operate on those fields before a page reload and crashed with a TypeError - which the service worker's 5xx-treated-as-offline fallback then masked as a misleading "you're offline" screen instead of a real error. Fixed by parsing with Django's parse_date/parse_time/parse_datetime at the point these values are read from the request/payload in tasks/views.py and sync/views.py, rather than trusting raw strings. 5 new regression tests exercise the actual views/endpoint with raw date/time strings and assert proper types land on the model. Co-Authored-By: Claude Sonnet 5 --- sync/tests.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ sync/views.py | 22 ++++++++++++++------- tasks/tests.py | 44 ++++++++++++++++++++++++++++++++++++++++++ tasks/views.py | 11 ++++++----- 4 files changed, 117 insertions(+), 12 deletions(-) diff --git a/sync/tests.py b/sync/tests.py index b84382f..21d112e 100644 --- a/sync/tests.py +++ b/sync/tests.py @@ -171,3 +171,55 @@ class SyncEndpointTests(TestCase): self.assertEqual(entry.notes, 'local notes') conflict.refresh_from_db() self.assertEqual(conflict.status, 'resolved_local') + + def test_creating_task_with_due_date_and_time_does_not_crash(self): + """ + Sync payloads carry due_date/due_time as JSON strings. Regression + test: these must end up as real date/time objects on the created + Task, not raw strings left for Task.reschedule_reminders() (called + from save()) to choke on. + """ + new_sync_id = str(uuid.uuid4()) + response = self.client.post('/api/sync/', { + 'device_id': 'web-test-device', + 'last_sync_token': None, + 'changes': { + 'tasks': [{ + 'sync_id': new_sync_id, + 'title': 'Due-dated task', + 'status': 'pending', + 'priority': 'medium', + 'due_date': '2026-09-10', + 'due_time': '17:00:00', + }], + 'tags': [], + 'time_entries': [], + }, + }, format='json') + + self.assertEqual(response.status_code, 200) + task = Task.objects.get(sync_id=new_sync_id) + self.assertEqual(task.due_date.isoformat(), '2026-09-10') + self.assertEqual(task.due_time.isoformat(), '17:00:00') + + def test_updating_task_due_date_and_time_does_not_crash(self): + task = Task.objects.create(user=self.user, title='Task to update') + + response = self.client.post('/api/sync/', { + 'device_id': 'web-test-device', + 'last_sync_token': None, + 'changes': { + 'tasks': [{ + 'sync_id': str(task.sync_id), + 'due_date': '2026-09-10', + 'due_time': '17:00:00', + }], + 'tags': [], + 'time_entries': [], + }, + }, format='json') + + self.assertEqual(response.status_code, 200) + task.refresh_from_db() + self.assertEqual(task.due_date.isoformat(), '2026-09-10') + self.assertEqual(task.due_time.isoformat(), '17:00:00') diff --git a/sync/views.py b/sync/views.py index 74e026a..b923a4f 100644 --- a/sync/views.py +++ b/sync/views.py @@ -8,7 +8,7 @@ import logging import uuid from datetime import datetime from django.utils import timezone -from django.utils.dateparse import parse_datetime +from django.utils.dateparse import parse_date, parse_datetime, parse_time from rest_framework import status from rest_framework.decorators import api_view, permission_classes, throttle_classes from rest_framework.permissions import IsAuthenticated @@ -289,6 +289,14 @@ def process_time_entry_changes(user, entry_changes, last_sync_at): return conflicts +def _parsed(data, key, default, parser): + """Get data[key], parsing it if it's still a raw string (e.g. from JSON), else default if absent.""" + if key not in data: + return default + value = data[key] + return parser(value) if isinstance(value, str) else value + + def update_task_from_data(task, data): """Update a task from sync data.""" # Track old status to detect completion @@ -298,9 +306,9 @@ def update_task_from_data(task, data): task.description = data.get('description', task.description) task.status = data.get('status', task.status) task.priority = data.get('priority', task.priority) - task.due_date = data.get('due_date', task.due_date) - task.due_time = data.get('due_time', task.due_time) - task.reminder_at = data.get('reminder_at', task.reminder_at) + task.due_date = _parsed(data, 'due_date', task.due_date, parse_date) + task.due_time = _parsed(data, 'due_time', task.due_time, parse_time) + task.reminder_at = _parsed(data, 'reminder_at', task.reminder_at, parse_datetime) task.recurrence = data.get('recurrence', task.recurrence) task.recurrence_rule = data.get('recurrence_rule', task.recurrence_rule) task.sort_order = data.get('sort_order', task.sort_order) @@ -341,9 +349,9 @@ def create_task_from_data(user, data): description=data.get('description', ''), status=data.get('status', 'pending'), priority=data.get('priority', 'medium'), - due_date=data.get('due_date'), - due_time=data.get('due_time'), - reminder_at=data.get('reminder_at'), + due_date=_parsed(data, 'due_date', None, parse_date), + due_time=_parsed(data, 'due_time', None, parse_time), + reminder_at=_parsed(data, 'reminder_at', None, parse_datetime), recurrence=data.get('recurrence', 'none'), recurrence_rule=data.get('recurrence_rule', ''), sort_order=data.get('sort_order', 0), diff --git a/tasks/tests.py b/tasks/tests.py index 9359a45..269dc51 100644 --- a/tasks/tests.py +++ b/tasks/tests.py @@ -133,3 +133,47 @@ class IsOverdueTests(TestCase): mock_now.return_value = django_timezone.make_aware(datetime(2026, 1, 15, 23, 0, 0)) task = self.make_task(due_date=date(2026, 1, 1), due_time=dt_time(9, 0, 0), status='completed') self.assertFalse(task.is_overdue) + + +class WebFormDueDateTypeTests(TestCase): + """ + The web views assign due_date/due_time straight from request.POST (raw + strings), relying on Django to coerce them at the DB layer - which + leaves the in-memory instance holding strings after save(). Anything + that touches those fields on that same instance afterward (e.g. + Task.reschedule_reminders(), called from save() itself) must not choke + on that. Regression test for a real production crash on task edit. + """ + + def setUp(self): + self.user = User.objects.create_user( + username='webformuser', email='webformuser@example.com', password='testpass123', + ) + self.client.force_login(self.user) + + def test_editing_due_date_and_time_does_not_crash(self): + task = Task.objects.create(user=self.user, title='Task') + response = self.client.post(f'/tasks/{task.id}/', { + 'title': 'Task', 'status': 'pending', 'priority': 'medium', + 'due_date': '2026-09-10', 'due_time': '17:00', 'recurrence': 'none', + }) + self.assertEqual(response.status_code, 302) + task.refresh_from_db() + self.assertEqual(task.due_date, date(2026, 9, 10)) + self.assertEqual(task.due_time, dt_time(17, 0)) + + def test_creating_task_with_due_date_does_not_crash(self): + response = self.client.post('/tasks/new/', { + 'title': 'New task', 'status': 'pending', 'priority': 'medium', + 'due_date': '2026-09-10', 'due_time': '17:00', 'recurrence': 'none', + }) + self.assertEqual(response.status_code, 302) + task = Task.objects.get(title='New task') + self.assertEqual(task.due_date, date(2026, 9, 10)) + self.assertEqual(task.due_time, dt_time(17, 0)) + + def test_quick_add_task_with_due_date_does_not_crash(self): + response = self.client.post('/tasks/quick-add/', {'title': 'Quick task', 'due_date': '2026-09-10'}) + self.assertEqual(response.status_code, 302) + task = Task.objects.get(title='Quick task') + self.assertEqual(task.due_date, date(2026, 9, 10)) diff --git a/tasks/views.py b/tasks/views.py index 9a033bb..8466c70 100644 --- a/tasks/views.py +++ b/tasks/views.py @@ -7,6 +7,7 @@ from django.contrib.auth.decorators import login_required from django.db.models import Q from django.shortcuts import render, redirect, get_object_or_404 from django.utils import timezone +from django.utils.dateparse import parse_date, parse_time from django.utils.http import url_has_allowed_host_and_scheme from django.views import View from django.views.decorators.http import require_POST @@ -466,10 +467,10 @@ class TaskDetailView(View): task.priority = priority due_date = request.POST.get('due_date') - task.due_date = due_date if due_date else None + task.due_date = parse_date(due_date) if due_date else None due_time = request.POST.get('due_time') - task.due_time = due_time if due_time else None + task.due_time = parse_time(due_time) if due_time else None # Validate recurrence against allowed choices recurrence = request.POST.get('recurrence', 'none') @@ -540,8 +541,8 @@ class TaskCreateView(View): description=request.POST.get('description', ''), status=status, priority=priority, - due_date=request.POST.get('due_date') or None, - due_time=request.POST.get('due_time') or None, + due_date=parse_date(request.POST.get('due_date')) if request.POST.get('due_date') else None, + due_time=parse_time(request.POST.get('due_time')) if request.POST.get('due_time') else None, recurrence=recurrence, recurrence_rule=request.POST.get('recurrence_rule', '') if recurrence == 'custom' else '', ) @@ -559,7 +560,7 @@ def task_quick_add(request): task = Task.objects.create( user=request.user, title=request.POST.get('title'), - due_date=request.POST.get('due_date') or None, + due_date=parse_date(request.POST.get('due_date')) if request.POST.get('due_date') else None, ) # Handle optional tag tag_id = request.POST.get('tag')