Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#24170 closed Cleanup/optimization (fixed)

DateTimeRangeField: widget doesn't implement decompress()

Reported by: Joel Burton Owned by: Ng Zhi An
Component: Database layer (models, ORM) Version: 1.8alpha1
Severity: Normal Keywords: DateTimeRangeField decompress
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

  1. Create a model with a DateTimeRangeField and another field (I used just one other, a SlugField)
  1. Via the admin, add a record with a valid slug and datetimerange
  1. Try to edit the record, changing the slug

(This was done with a Postgres backend)

This traceback is produced:

Environment:


Request Method: POST
Request URL: http://localhost:8000/admin/homework/homework/3/

Django Version: 1.8a1
Python Version: 3.4.2
Installed Applications:
('django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.postgres',
 'homework')
Installed Middleware:
('django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.auth.middleware.SessionAuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'django.middleware.security.SecurityMiddleware')


Traceback:
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/core/handlers/base.py" in get_response
  131.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/contrib/admin/options.py" in wrapper
  615.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/utils/decorators.py" in _wrapped_view
  110.                     response = view_func(request, *args, **kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  54.         response = view_func(request, *args, **kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/contrib/admin/sites.py" in inner
  226.             return view(request, *args, **kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/contrib/admin/options.py" in change_view
  1518.         return self.changeform_view(request, object_id, form_url, extra_context)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/utils/decorators.py" in _wrapper
  34.             return bound_func(*args, **kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/utils/decorators.py" in _wrapped_view
  110.                     response = view_func(request, *args, **kwargs)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/utils/decorators.py" in bound_func
  30.                 return func.__get__(self, type(self))(*args2, **kwargs2)
File "/opt/local/Library/Frameworks/Python.framework/Versions/3.4/lib/python3.4/contextlib.py" in inner
  30.                 return func(*args, **kwds)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/contrib/admin/options.py" in changeform_view
  1472.                     change_message = self.construct_change_message(request, form, formsets)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/contrib/admin/options.py" in construct_change_message
  1020.         if form.changed_data:
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/forms/forms.py" in changed_data
  466.                 if field.has_changed(initial_value, data_value):
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/forms/fields.py" in has_changed
  1126.                 initial = self.widget.decompress(initial)
File "/Users/joel/programming/django18play/venv/lib/python3.4/site-packages/django/forms/widgets.py" in decompress
  850.         raise NotImplementedError('Subclasses must implement this method.')

Exception Type: NotImplementedError at /admin/homework/homework/3/
Exception Value: Subclasses must implement this method.

Change History (8)

comment:1 by Ng Zhi An, 10 years ago

Owner: changed from nobody to Ng Zhi An
Status: newassigned

comment:2 by Ng Zhi An, 10 years ago

Error is caused because all the new fields added in 1.8 IntegerRangeField, FloatRangeField, DateTimeRangeField, and DateRangeField, all inherit from BaseRangeField.

BaseRangeField itself defines its widget as forms.MultiWidget([self.base_field.widget, self.base_field.widget]) postgres.forms.ranges.py#18. However, MultiWidget raises NotImplementedError('Subclasses must implement this method.') just like @joelburton mentioned.

I'm not sure how to patch this. Thinking that:

  1. Each of the child classes of BaseRangeField should overwrite __init__ to put in their own widgets.
  2. create a new widgets.py file in contrib.postgres to define the new widgets.

Comments?

Version 0, edited 10 years ago by Ng Zhi An (next)

in reply to:  2 ; comment:3 by Simon Charette, 10 years ago

Triage Stage: UnreviewedAccepted

I think creating a single RangeWidget(widgets.Widget) class in a new widgets module that implements the decompress method should do.

# django/contrib/postgres/widgets.py

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

    def decompress(self, value):
        if value:
          return (value.lower, value.upper)
        return (None, None)

Please make a PR of your patch to make review easier. Thanks!

in reply to:  3 ; comment:4 by Ng Zhi An, 10 years ago

Replying to charettes:

I think creating a single RangeWidget(widgets.Widget) class in a new widgets module that implements the decompress method should do.

Implemented it and submitted a PR, it's lacking test though, I'm not too sure how to test it. Will creating a field of each type IntegerRangeField, FloatRangeField, DateTimeRangeField, and DateRangeField and asserting self.widget.decompress() be sufficient?

in reply to:  4 comment:5 by Markus Holtermann, 10 years ago

Has patch: set
Needs documentation: set
Needs tests: set
Type: UncategorizedCleanup/optimization

Replying to ngzhian:

Implemented it and submitted a PR, it's lacking test though, I'm not too sure how to test it. Will creating a field of each type IntegerRangeField, FloatRangeField, DateTimeRangeField, and DateRangeField and asserting self.widget.decompress() be sufficient?

I'd have a look at the tests of the "normal" widgets if there aren't any tests yet. I don't think there is a need to tests all range fields.

comment:6 by Markus Holtermann, 10 years ago

Needs documentation: unset
Needs tests: unset
Triage Stage: AcceptedReady for checkin

comment:7 by Ng Zhi An <ngzhian@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 4669b6a807811d6763b9fdc5df974cb67aa1fb56:

Fixed #24170 -- Implemented decompress for BaseRangeField widgets

comment:8 by Tim Graham <timograham@…>, 10 years ago

In 56015c01c4725aeeb225a62f2702a4bbb3a3ce54:

[1.8.x] Fixed #24170 -- Implemented decompress for BaseRangeField widgets

Backport of 4669b6a807811d6763b9fdc5df974cb67aa1fb56 from master

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