Opened 9 years ago

Closed 9 years ago

Last modified 9 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, 9 years ago

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

comment:2 by Ng Zhi An, 9 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.

Created a preliminary patch here

Comments?

Last edited 9 years ago by Ng Zhi An (previous) (diff)

in reply to:  2 ; comment:3 by Simon Charette, 9 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, 9 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, 9 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, 9 years ago

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

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

Resolution: fixed
Status: assignedclosed

In 4669b6a807811d6763b9fdc5df974cb67aa1fb56:

Fixed #24170 -- Implemented decompress for BaseRangeField widgets

comment:8 by Tim Graham <timograham@…>, 9 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