Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30758 closed Bug (fixed)

DateTimeRangeField with default crashes in django admin (object has no attribute 'strip').

Reported by: yeppus Owned by: Nasir Hussain
Component: contrib.postgres Version: dev
Severity: Normal Keywords: DateTimeRangeField
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Mariusz Felisiak)

When trying to save an object that has a DateTimeRangeField in django admin the following error occurs:
AttributeError: 'builtin_function_or_method' object has no attribute 'strip'

When trying to determine if the value has changed it, maybe accidentally, assigns initial value the function "lower" instead of the value.

Later it tries to run .strip() on the function.

This all worked in django 1.11 and I can not find any mentioned of changed behaviour for Django 2.2.

django/contrib/postgres/forms/ranges.py: 101

Code highlighting: 

class RangeWidget(MultiWidget):
  def __init__(self, base_widget, attrs=None):
      widgets = (base_widget, base_widget)
      super().__init__(widgets, attrs)

  def decompress(self, value):
      if value:
          return (value.lower, value.upper) ### <<-- RETURNS CALLABLE, NOT VALUE
      return (None, None)

django/forms/fields.py: 1060

Code highlighting: 

  def has_changed(self, initial, data):
      if self.disabled:
          return False
      if initial is None:
          initial = ['' for x in range(0, len(data))]
      else:
          if not isinstance(initial, list):
              initial = self.widget.decompress(initial) ### <<-- RECEIVES CALLABLE, NOT VALUE
      for field, initial, data in zip(self.fields, initial, data):
          try:
              initial = field.to_python(initial) ### <<-- TRIES to_python with CALLABLE
          except ValidationError:
              return True
          if field.has_changed(initial, data):
              return True
      return False

django/forms/fields.py: 450

Code highlighting: 

  def to_python(self, value):
      """
      Validate that the input can be converted to a datetime. Return a
      Python datetime.datetime object.
      """
      if value in self.empty_values:
          return None
      if isinstance(value, datetime.datetime):
          return from_current_timezone(value)
      if isinstance(value, datetime.date):
          result = datetime.datetime(value.year, value.month, value.day)
          return from_current_timezone(result)
      result = super().to_python(value) ### <<-- ENDS UP HERE SENDING CALLABLE TO PARENT
      return from_current_timezone(result)

BaseTemporalField.to_python expects a string and runs .strip() which generates AttributeError and crashes.

Traceback (most recent call last):
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/contrib/admin/options.py", line 606, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/contrib/admin/sites.py", line 223, in inner
    return view(request, *args, **kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/contrib/admin/options.py", line 1637, in change_view
    return self.changeform_view(request, object_id, form_url, extra_context)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/utils/decorators.py", line 45, in _wrapper
    return bound_method(*args, **kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/contrib/admin/options.py", line 1522, in changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/contrib/admin/options.py", line 1560, in _changeform_view
    if all_valid(formsets) and form_validated:
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/formsets.py", line 448, in all_valid
    valid &= formset.is_valid()
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/formsets.py", line 301, in is_valid
    self.errors
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/formsets.py", line 281, in errors
    self.full_clean()
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/formsets.py", line 325, in full_clean
    if not form.has_changed() and i >= self.initial_form_count():
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/contrib/admin/options.py", line 2111, in has_changed
    return super().has_changed()
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/forms.py", line 434, in has_changed
    return bool(self.changed_data)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/utils/functional.py", line 80, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/forms.py", line 456, in changed_data
    if field.has_changed(initial_value, data_value):
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/fields.py", line 1070, in has_changed
    initial = field.to_python(initial)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/fields.py", line 462, in to_python
    result = super().to_python(value)
  File "/Users/joakim/.pyenv/versions/3.5.2/lib/python3.5/site-packages/django/forms/fields.py", line 379, in to_python
    value = value.strip()

Change History (14)

comment:1 by Mariusz Felisiak, 5 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 5 years ago

Resolution: → needsinfo
Status: new → closed
Summary: Postgres DateTimeRangeField crash in django admin (AttributeError: 'builtin_function_or_method' object has no attribute 'strip') → DateTimeRangeField crashes in django admin (object has no attribute 'strip').
Version: 2.2 → master

value.lower and value.upper are not callable they are lower and upper bound of a range. I'm not able to reproduce this issue in Django 2.2 or on a current master. It works for me with inlines and a direct form. Please provide a sample (minimal) project to reproduce this issue.

comment:3 by yeppus, 5 years ago

Resolution: needsinfo
Status: closed → new

It seems to be connected to default function. After some digging and testing I managed to make a minimal testcase.

admin.py

Code highlighting: 

from django.contrib import admin
from . import models


@admin.register(models.DateRangeWithDefaultFunction)
class PriceAdmin(admin.ModelAdmin):
    fields = (
        'valid_period',
    )
    list_display = (
        '__str__',
    )


@admin.register(models.DateRangeWithoutDefaultFunction)
class PriceAdmin(admin.ModelAdmin):
    fields = (
        'valid_period',
    )
    list_display = (
        '__str__',
    )

models.py

Code highlighting: 

from django.db import models
from django.utils import timezone
from django.contrib.postgres import fields as pg_fields
from psycopg2._range import DateTimeTZRange


def default_period():
    return DateTimeTZRange(
        lower=timezone.now(),
        upper=None,
    )

#Can add new but not change objects in admin
class DateRangeWithDefaultFunction(models.Model):

    valid_period = pg_fields.DateTimeRangeField(
        'valid during',
        blank=True,
        default=default_period,
    )

#Working as expected
class DateRangeWithoutDefaultFunction(models.Model):
    valid_period = pg_fields.DateTimeRangeField(
        'valid during',
        blank=True,
    )

comment:4 by Mariusz Felisiak, 5 years ago

Summary: DateTimeRangeField crashes in django admin (object has no attribute 'strip'). → DateTimeRangeField with default crashes in django admin (object has no attribute 'strip').
Triage Stage: Unreviewed → Accepted

Thanks for info! I can confirm that this issue is caused by default.

comment:5 by Nasir Hussain, 5 years ago

Owner: set to Nasir Hussain
Status: new → assigned

comment:6 by Nasir Hussain, 5 years ago

Has patch: set

I've created a ​PR.

comment:7 by Claude Paroz, 5 years ago

Needs tests: set

Thanks for the PR, now a test is still required.

in reply to:  7 comment:8 by Nasir Hussain, 5 years ago

Needs tests: unset

Replying to Claude Paroz:

Thanks for the PR, now a test is still required.

Added a test case which fails in case of master and passes with the fix.

comment:9 by yeppus, 5 years ago

Hi All, I can confirm that this patch works on our product staging environment where we previously encounter the issue as well.

comment:10 by Mariusz Felisiak, 5 years ago

Needs tests: set
Patch needs improvement: set

comment:11 by Nasir Hussain, 5 years ago

Needs tests: unset
Patch needs improvement: unset

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 733dbb2:

Refs #30758 -- Added more tests for postgres.forms.ranges.

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: → fixed
Status: assigned → closed

In faf4b988:

Fixed #30758 -- Made RangeFields use multiple hidden inputs for initial data.

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 685d9567:

[3.0.x] Fixed #30758 -- Made RangeFields use multiple hidden inputs for initial data.

Backport of faf4b988fe75dd4045bc5c62496cc4f2e0db8c4d from master.

Note: See TracTickets for help on using tickets.
Back to Top