Internal
Public Access
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
5359bf243a
commit
a27fcc4c69
@@ -171,3 +171,55 @@ class SyncEndpointTests(TestCase):
|
|||||||
self.assertEqual(entry.notes, 'local notes')
|
self.assertEqual(entry.notes, 'local notes')
|
||||||
conflict.refresh_from_db()
|
conflict.refresh_from_db()
|
||||||
self.assertEqual(conflict.status, 'resolved_local')
|
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')
|
||||||
|
|||||||
+15
-7
@@ -8,7 +8,7 @@ import logging
|
|||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from django.utils import timezone
|
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 import status
|
||||||
from rest_framework.decorators import api_view, permission_classes, throttle_classes
|
from rest_framework.decorators import api_view, permission_classes, throttle_classes
|
||||||
from rest_framework.permissions import IsAuthenticated
|
from rest_framework.permissions import IsAuthenticated
|
||||||
@@ -289,6 +289,14 @@ def process_time_entry_changes(user, entry_changes, last_sync_at):
|
|||||||
return conflicts
|
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):
|
def update_task_from_data(task, data):
|
||||||
"""Update a task from sync data."""
|
"""Update a task from sync data."""
|
||||||
# Track old status to detect completion
|
# 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.description = data.get('description', task.description)
|
||||||
task.status = data.get('status', task.status)
|
task.status = data.get('status', task.status)
|
||||||
task.priority = data.get('priority', task.priority)
|
task.priority = data.get('priority', task.priority)
|
||||||
task.due_date = data.get('due_date', task.due_date)
|
task.due_date = _parsed(data, 'due_date', task.due_date, parse_date)
|
||||||
task.due_time = data.get('due_time', task.due_time)
|
task.due_time = _parsed(data, 'due_time', task.due_time, parse_time)
|
||||||
task.reminder_at = data.get('reminder_at', task.reminder_at)
|
task.reminder_at = _parsed(data, 'reminder_at', task.reminder_at, parse_datetime)
|
||||||
task.recurrence = data.get('recurrence', task.recurrence)
|
task.recurrence = data.get('recurrence', task.recurrence)
|
||||||
task.recurrence_rule = data.get('recurrence_rule', task.recurrence_rule)
|
task.recurrence_rule = data.get('recurrence_rule', task.recurrence_rule)
|
||||||
task.sort_order = data.get('sort_order', task.sort_order)
|
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', ''),
|
description=data.get('description', ''),
|
||||||
status=data.get('status', 'pending'),
|
status=data.get('status', 'pending'),
|
||||||
priority=data.get('priority', 'medium'),
|
priority=data.get('priority', 'medium'),
|
||||||
due_date=data.get('due_date'),
|
due_date=_parsed(data, 'due_date', None, parse_date),
|
||||||
due_time=data.get('due_time'),
|
due_time=_parsed(data, 'due_time', None, parse_time),
|
||||||
reminder_at=data.get('reminder_at'),
|
reminder_at=_parsed(data, 'reminder_at', None, parse_datetime),
|
||||||
recurrence=data.get('recurrence', 'none'),
|
recurrence=data.get('recurrence', 'none'),
|
||||||
recurrence_rule=data.get('recurrence_rule', ''),
|
recurrence_rule=data.get('recurrence_rule', ''),
|
||||||
sort_order=data.get('sort_order', 0),
|
sort_order=data.get('sort_order', 0),
|
||||||
|
|||||||
@@ -133,3 +133,47 @@ class IsOverdueTests(TestCase):
|
|||||||
mock_now.return_value = django_timezone.make_aware(datetime(2026, 1, 15, 23, 0, 0))
|
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')
|
task = self.make_task(due_date=date(2026, 1, 1), due_time=dt_time(9, 0, 0), status='completed')
|
||||||
self.assertFalse(task.is_overdue)
|
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))
|
||||||
|
|||||||
+6
-5
@@ -7,6 +7,7 @@ from django.contrib.auth.decorators import login_required
|
|||||||
from django.db.models import Q
|
from django.db.models import Q
|
||||||
from django.shortcuts import render, redirect, get_object_or_404
|
from django.shortcuts import render, redirect, get_object_or_404
|
||||||
from django.utils import timezone
|
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.utils.http import url_has_allowed_host_and_scheme
|
||||||
from django.views import View
|
from django.views import View
|
||||||
from django.views.decorators.http import require_POST
|
from django.views.decorators.http import require_POST
|
||||||
@@ -466,10 +467,10 @@ class TaskDetailView(View):
|
|||||||
task.priority = priority
|
task.priority = priority
|
||||||
|
|
||||||
due_date = request.POST.get('due_date')
|
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')
|
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
|
# Validate recurrence against allowed choices
|
||||||
recurrence = request.POST.get('recurrence', 'none')
|
recurrence = request.POST.get('recurrence', 'none')
|
||||||
@@ -540,8 +541,8 @@ class TaskCreateView(View):
|
|||||||
description=request.POST.get('description', ''),
|
description=request.POST.get('description', ''),
|
||||||
status=status,
|
status=status,
|
||||||
priority=priority,
|
priority=priority,
|
||||||
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,
|
||||||
due_time=request.POST.get('due_time') or None,
|
due_time=parse_time(request.POST.get('due_time')) if request.POST.get('due_time') else None,
|
||||||
recurrence=recurrence,
|
recurrence=recurrence,
|
||||||
recurrence_rule=request.POST.get('recurrence_rule', '') if recurrence == 'custom' else '',
|
recurrence_rule=request.POST.get('recurrence_rule', '') if recurrence == 'custom' else '',
|
||||||
)
|
)
|
||||||
@@ -559,7 +560,7 @@ def task_quick_add(request):
|
|||||||
task = Task.objects.create(
|
task = Task.objects.create(
|
||||||
user=request.user,
|
user=request.user,
|
||||||
title=request.POST.get('title'),
|
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
|
# Handle optional tag
|
||||||
tag_id = request.POST.get('tag')
|
tag_id = request.POST.get('tag')
|
||||||
|
|||||||
Reference in New Issue
Block a user