Skip to content

Commit

Permalink
Remove the SCHEDULER_BASE_CLASSES feature
Browse files Browse the repository at this point in the history
The feature is brittle with buggy edge cases. Use cases can be better
solved through other methods such as the EventRelation model. Reduce the
large maintenance burden by removing the feature. Results in cleaner
code that follows Django reusable app conventions.

Closes #357
Closes #310
Closes #306
  • Loading branch information
jdufresne committed Jan 7, 2018
1 parent a56c1b7 commit 90de9c6
Show file tree
Hide file tree
Showing 9 changed files with 12 additions and 202 deletions.
38 changes: 0 additions & 38 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,44 +176,6 @@ Default:
return calendar.event_set.all()
```

### SCHEDULER_BASE_CLASSES

This setting allows for custom base classes to be used on specific models. Useful for adding fields, managers, or other elements.

Defaults to django.db.models.Model

Extend all the models using a list:

```
SCHEDULER_BASE_CLASSES = ['my_app.models.BaseAbstract1', 'my_app.models.BaseAbstract1']
```

Extend specific models using a dict, where the key is the model class name:

```
SCHEDULER_BASE_CLASSES = {
'Event': ['my_app.models.EventAbstract1', 'my_app..models.EventAbstract2']
'Calendar': [my_app.models.CalendarAbstract']
}
```

### SCHEDULER_ADMIN_FIELDS
This complements the SCHEDULER_BASE_CLASSES feature, by allowing fields
added via a base class to be shown in the admin form for that model.

Example - EventBase adds a 'cost' field, and now the field can be shown
in the admin form too.

```
SCHEDULER_BASE_CLASSES = {
'Event': ['main.models.EventBase']
}
SCHEDULER_ADMIN_FIELDS = {
'Event': [('cost',)]
}
### SCHEDULER_PREVNEXT_LIMIT_SECONDS

This settings allows to set the upper and lower limit in calendars navigation.
Expand Down
43 changes: 0 additions & 43 deletions docs/settings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,46 +57,3 @@ example::

get_events(request, calendar):
return calendar.event_set.all()


SCHEDULER_BASE_CLASSES
---------------

This setting allows for custom base classes to be used on specific models. Useful for adding fields, managers, or other elements.

Defaults to django.db.models.Model

Extend all the models using a list::

SCHEDULER_BASE_CLASSES = ['my_app.models.BaseAbstract1', 'my_app.models.BaseAbstract1']


Extend specific models using a dict, where the key is the model class name::

SCHEDULER_BASE_CLASSES = {
'Event': ['my_app.models.EventAbstract1', 'my_app..models.EventAbstract2']
'Calendar': [my_app.models.CalendarAbstract']
}

.. _ref-settings-scheduler-base-classes:


SCHEDULER_ADMIN_FIELDS
---------------

This complements the SCHEDULER_BASE_CLASSES feature, by allowing fields
added via a base class to be shown in the admin form for that model.

Example - EventBase adds a 'cost' field, and now the field can be shown
in the admin form too.

```
SCHEDULER_BASE_CLASSES = {
'Event': ['main.models.EventBase']
}

SCHEDULER_ADMIN_FIELDS = {
'Event': [('cost',)]
}

.. _ref-settings-scheduler-admin-fields
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ Django>=1.11
flake8
icalendar>=3.8.4
isort
mock
python-dateutil>=2.1
pytz>=2013.9
wheel
5 changes: 2 additions & 3 deletions schedule/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

from schedule.forms import EventAdminForm
from schedule.models import Calendar, CalendarRelation, Event, Occurrence, Rule
from schedule.utils import get_admin_model_fields


class CalendarAdminOptions(admin.ModelAdmin):
Expand All @@ -13,7 +12,7 @@ class CalendarAdminOptions(admin.ModelAdmin):
(None, {
'fields': [
('name', 'slug'),
] + get_admin_model_fields('Calendar')
]
}),
)

Expand Down Expand Up @@ -46,7 +45,7 @@ class EventAdmin(admin.ModelAdmin):
('start', 'end'),
('creator', 'calendar'),
('rule', 'end_recurring_period'),
] + get_admin_model_fields('Event')
]
}),
)
form = EventAdminForm
Expand Down
8 changes: 3 additions & 5 deletions schedule/models/calendars.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.db.models import Q
from django.db.models.base import ModelBase
from django.template.defaultfilters import slugify
from django.urls import reverse
from django.utils import timezone
from django.utils.encoding import python_2_unicode_compatible
from django.utils.six import with_metaclass
from django.utils.six.moves.builtins import str
from django.utils.translation import ugettext_lazy as _

from schedule.settings import USE_FULLCALENDAR
from schedule.utils import EventListManager, get_model_bases
from schedule.utils import EventListManager


class CalendarManager(models.Manager):
Expand Down Expand Up @@ -106,7 +104,7 @@ def get_calendars_for_object(self, obj, distinction=''):


@python_2_unicode_compatible
class Calendar(with_metaclass(ModelBase, *get_model_bases('Calendar'))):
class Calendar(models.Model):
'''
This is for grouping events so that batch relations can be made to all
events. An example would be a project calendar.
Expand Down Expand Up @@ -201,7 +199,7 @@ def create_relation(self, calendar, content_object, distinction='', inheritable=


@python_2_unicode_compatible
class CalendarRelation(with_metaclass(ModelBase, *get_model_bases('CalendarRelation'))):
class CalendarRelation(models.Model):
'''
This is for relating data to a Calendar, and possible all of the events for
that calendar, there is also a distinction, so that the same type or kind of
Expand Down
10 changes: 4 additions & 6 deletions schedule/models/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,16 @@
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.db.models import Q
from django.db.models.base import ModelBase
from django.template.defaultfilters import date
from django.urls import reverse
from django.utils import timezone
from django.utils.encoding import python_2_unicode_compatible
from django.utils.six import with_metaclass
from django.utils.translation import ugettext_lazy as _
from django.utils.translation import ugettext

from schedule.models.calendars import Calendar
from schedule.models.rules import Rule
from schedule.utils import OccurrenceReplacer, get_model_bases
from schedule.utils import OccurrenceReplacer

freq_dict_order = {
'YEARLY': 0,
Expand Down Expand Up @@ -49,7 +47,7 @@ def get_for_object(self, content_object, distinction='', inherit=True):


@python_2_unicode_compatible
class Event(with_metaclass(ModelBase, *get_model_bases('Event'))):
class Event(models.Model):
'''
This model stores meta data for a date. You can relate this data to many
other models.
Expand Down Expand Up @@ -527,7 +525,7 @@ def create_relation(self, event, content_object, distinction=''):


@python_2_unicode_compatible
class EventRelation(with_metaclass(ModelBase, *get_model_bases('EventRelation'))):
class EventRelation(models.Model):
'''
This is for relating data to an Event, there is also a distinction, so that
data can be related in different ways. A good example would be, if you have
Expand Down Expand Up @@ -563,7 +561,7 @@ def __str__(self):


@python_2_unicode_compatible
class Occurrence(with_metaclass(ModelBase, *get_model_bases('Occurrence'))):
class Occurrence(models.Model):
event = models.ForeignKey(Event, on_delete=models.CASCADE, verbose_name=_("event"))
title = models.CharField(_("title"), max_length=255, blank=True)
description = models.TextField(_("description"), blank=True)
Expand Down
6 changes: 1 addition & 5 deletions schedule/models/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,10 @@
WEEKLY, YEARLY,
)
from django.db import models
from django.db.models.base import ModelBase
from django.utils.encoding import python_2_unicode_compatible
from django.utils.six import with_metaclass
from django.utils.six.moves.builtins import str
from django.utils.translation import ugettext_lazy as _

from schedule.utils import get_model_bases

freqs = (("YEARLY", _("Yearly")),
("MONTHLY", _("Monthly")),
("WEEKLY", _("Weekly")),
Expand All @@ -23,7 +19,7 @@


@python_2_unicode_compatible
class Rule(with_metaclass(ModelBase, *get_model_bases('Rule'))):
class Rule(models.Model):
"""
This defines a rule by which an event will recur. This is defined by the
rrule in the dateutil documentation.
Expand Down
28 changes: 0 additions & 28 deletions schedule/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from django.conf import settings
from django.http import HttpResponseNotFound, HttpResponseRedirect
from django.utils import timezone
from django.utils.module_loading import import_string

from schedule.settings import (
CALENDAR_VIEW_PERM, CHECK_CALENDAR_PERM_FUNC, CHECK_EVENT_PERM_FUNC,
Expand Down Expand Up @@ -218,30 +217,3 @@ def coerce_date_dict(date_dict):
except KeyError:
break
return modified and ret_val or {}


def get_model_bases(model_class_name):
from django.db.models import Model
base_classes = getattr(settings, 'SCHEDULER_BASE_CLASSES', {})

if isinstance(base_classes, dict):
base_class_names = base_classes.get(model_class_name, [])
else:
base_class_names = base_classes

if base_class_names:
return [import_string(x) for x in base_class_names]
else:
return [Model]


def get_admin_model_fields(model_class_name):
admin_fields = getattr(settings, 'SCHEDULER_ADMIN_FIELDS', {})
if isinstance(admin_fields, dict):
model_fields = admin_fields.get(model_class_name, [])
else:
model_fields = admin_fields
if model_fields:
return model_fields
else:
return []
75 changes: 2 additions & 73 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
import datetime

import mock
from django.test import TestCase, override_settings
from django.test import TestCase
from django.utils import timezone

from schedule.models import Calendar, Event, Occurrence, Rule
from schedule.utils import (
EventListManager, OccurrenceReplacer, get_admin_model_fields,
get_model_bases,
)
from schedule.utils import EventListManager, OccurrenceReplacer


class TestEventListManager(TestCase):
Expand Down Expand Up @@ -140,70 +136,3 @@ def test_get_occurrence_works_for_event_like_object(self):
occ_replacer = OccurrenceReplacer([self.occ])
with self.assertRaises(AttributeError):
occ_replacer.get_occurrence(int)


class TestCommonUtils(TestCase):
def setUp(self):
patcher = mock.patch('schedule.utils.import_string')
self.import_string_mock = patcher.start()
self.addCleanup(patcher.stop)

@override_settings(SCHEDULER_BASE_CLASSES={'ClassName': ['path.to.module.AbstractClass']})
def test_get_model_bases_with_custom_dict_default(self):
from django.db.models import Model
expected_result = [Model]
actual_result = get_model_bases('Event')

self.assertListEqual(actual_result, expected_result)

@override_settings(SCHEDULER_BASE_CLASSES={'ClassName': ['path.to.module.AbstractClass']})
def test_get_model_bases_with_custom_dict_specific(self):
model_mock = mock.Mock()
expected_result = [model_mock]

self.import_string_mock.return_value = model_mock
actual_result = get_model_bases('ClassName')

self.assertListEqual(actual_result, expected_result)

self.import_string_mock.assert_called_once_with('path.to.module.AbstractClass')

@override_settings(SCHEDULER_BASE_CLASSES=['path.to.module.AbstractClass1', 'path.to.module.AbstractClass2'])
def test_get_model_bases_with_custom_list(self):
model_mock1 = mock.Mock()
model_mock2 = mock.Mock()

expected_result = [model_mock1, model_mock2]

self.import_string_mock.side_effect = [model_mock1, model_mock2]
actual_result = get_model_bases('ClassName')

self.assertListEqual(actual_result, expected_result)

self.import_string_mock.assert_any_call('path.to.module.AbstractClass1')
self.import_string_mock.assert_any_call('path.to.module.AbstractClass2')
self.assertEqual(self.import_string_mock.call_count, 2)

@override_settings(SCHEDULER_BASE_CLASSES=None)
def test_get_model_bases_with_no_setting(self):
from django.db.models import Model
expected_result = [Model]
actual_result = get_model_bases('Event')

self.assertListEqual(actual_result, expected_result)

@override_settings(SCHEDULER_ADMIN_FIELDS=[('cost',)])
def test_get_admin_fields_with_custom_list(self):
self.assertListEqual(get_admin_model_fields('Event'), [('cost',)])

@override_settings(SCHEDULER_ADMIN_FIELDS={'ClassName': [('cost',)]})
def test_get_admin_fields_with_custom_dict_specific(self):
self.assertListEqual(get_admin_model_fields('ClassName'), [('cost',)])

@override_settings(SCHEDULER_ADMIN_FIELDS={'ClassName': [('cost',)]})
def test_get_admin_fields_with_custom_dict_default(self):
self.assertListEqual(get_admin_model_fields('Event'), [])

@override_settings(SCHEDULER_ADMIN_FIELDS=None)
def test_get_admin_fields_with_no_setting(self):
self.assertListEqual(get_admin_model_fields('Event'), [])

0 comments on commit 90de9c6

Please sign in to comment.