Skip to content

Commit

Permalink
Fix bug with deconstruction when using choices, choices_display
Browse files Browse the repository at this point in the history
Fixes #74
  • Loading branch information
mfogel committed Jul 6, 2021
1 parent fa3bc50 commit 121ed9c
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 8 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ poetry run pytest
#### Unreleased
* Officially support for django 3.2, python 3.9
* Fix bug with field deconstruction ([#74](https://github.com/mfogel/django-timezone-field/issues/74))
#### 4.1.2 (2021-03-17)
Expand Down
57 changes: 57 additions & 0 deletions tests/test_deconstruct.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import pytest
import pytz

from django.db.migrations.writer import MigrationWriter
Expand Down Expand Up @@ -95,3 +96,59 @@ def test_specifying_defaults_not_frozen():
field = TimeZoneField(choices=choices)
name, path, args, kwargs = field.deconstruct()
assert 'choices' not in kwargs


@pytest.mark.parametrize('choices', [[
(pytz.timezone('US/Pacific'), 'US/Pacific'),
(pytz.timezone('US/Eastern'), 'US/Eastern'),
], [
('US/Pacific', 'US/Pacific'),
('US/Eastern', 'US/Eastern'),
]])
def test_deconstruct_when_using_just_choices(choices):
field = TimeZoneField(choices=choices)
name, path, args, kwargs = field.deconstruct()
assert kwargs == {'choices': [
('US/Pacific', 'US/Pacific'),
('US/Eastern', 'US/Eastern'),
]}


@pytest.mark.parametrize('choices_display, expected_kwargs', [
[None, {}],
['STANDARD', {'choices_display': 'STANDARD'}],
['WITH_GMT_OFFSET', {'choices_display': 'WITH_GMT_OFFSET'}],
])
def test_deconstruct_when_using_just_choices_display(choices_display, expected_kwargs):
field = TimeZoneField(choices_display=choices_display)
name, path, args, kwargs = field.deconstruct()
assert kwargs == expected_kwargs


@pytest.mark.parametrize('choices, choices_display, expected_kwargs', [
[
[['US/Pacific', 'West Coast Time']],
None,
{'choices': [('US/Pacific', 'West Coast Time')]},
],
[
[['US/Pacific', 'West Coast Time']],
'STANDARD',
{'choices': [('US/Pacific', '')], 'choices_display': 'STANDARD'},
],
[
[['US/Pacific', 'West Coast Time']],
'WITH_GMT_OFFSET',
{'choices': [('US/Pacific', '')], 'choices_display': 'WITH_GMT_OFFSET'},
],
[
[[tz, 'ignored'] for tz in pytz.common_timezones],
'WITH_GMT_OFFSET',
{'choices_display': 'WITH_GMT_OFFSET'},
],
])
def test_deconstruct_when_using_choices_and_choices_display(
choices, choices_display, expected_kwargs):
field = TimeZoneField(choices=choices, choices_display=choices_display)
name, path, args, kwargs = field.deconstruct()
assert kwargs == expected_kwargs
27 changes: 19 additions & 8 deletions timezone_field/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ class TimeZoneField(models.Field):
# NOTE: these defaults are excluded from migrations. If these are changed,
# existing migration files will need to be accomodated.
default_tzs = [pytz.timezone(tz) for tz in pytz.common_timezones]
default_choices = standard(default_tzs)
default_max_length = 63

def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -67,17 +66,17 @@ def __init__(self, *args, **kwargs):
values = self.default_tzs
displays = None

choices_display = kwargs.pop('choices_display', None)
if choices_display == 'WITH_GMT_OFFSET':
self.choices_display = kwargs.pop('choices_display', None)
if self.choices_display == 'WITH_GMT_OFFSET':
choices = with_gmt_offset(values)
elif choices_display == 'STANDARD':
elif self.choices_display == 'STANDARD':
choices = standard(values)
elif choices_display is None:
elif self.choices_display is None:
choices = zip(values, displays) if displays else standard(values)
else:
raise ValueError(
"Unrecognized value for kwarg 'choices_display' of '"
+ choices_display + "'"
+ self.choices_display + "'"
)

# 'display_GMT_offset' is deprecated, use 'choices_display' instead
Expand All @@ -94,11 +93,23 @@ def validate(self, value, model_instance):

def deconstruct(self):
name, path, args, kwargs = super(TimeZoneField, self).deconstruct()
if kwargs.get('choices') == self.default_choices:
del kwargs['choices']
if kwargs.get('max_length') == self.default_max_length:
del kwargs['max_length']

if self.choices_display is not None:
kwargs['choices_display'] = self.choices_display

choices = kwargs['choices']
if self.choices_display is None:
if choices == standard(self.default_tzs):
kwargs.pop('choices')
else:
values, _ = zip(*choices)
if sorted(values, key=str) == sorted(self.default_tzs, key=str):
kwargs.pop('choices')
else:
kwargs['choices'] = [(value, '') for value in values]

# django can't decontruct pytz objects, so transform choices
# to [<str>, <str>] format for writing out to the migration
if 'choices' in kwargs:
Expand Down

0 comments on commit 121ed9c

Please sign in to comment.