diff --git a/README b/README index f132a19..1f6f71a 100644 --- a/README +++ b/README @@ -46,8 +46,9 @@ have an inner Meta class that inherits from ``Sortable.Meta`` For models that you want sortable relative to a ``ForeignKey`` field, you need to -specify an ``@classmethod`` that returns a double: the foreign key class, and the -name of the foreign key property as defined on your model, as a string. +specify a property: ``sortable_by`` that is equal to the class defined as your ForeignKey field. +If you're upgrading from a version < 1.2, you do not need to redefine sortable_by. +1.2 is backwards compatible to 1.0. #admin.py class Category(models.Model): @@ -63,9 +64,7 @@ name of the foreign key property as defined on your model, as a string. def __unicode__(self): return self.title - @classmethod - def sortable_by(cls): - return Category, 'category' + sortable_by = Category Sortable has one field: `order` and adds a default ordering value set to `order`. @@ -132,7 +131,16 @@ Status ============= admin-sortable is currently used in production. -Feautures + +What's new in 1.2 +============= +- Refactored ``sortable_by`` to be a property rather than a classmethod, which is much less work to implement. +- Fixed an issue with ordering which could result in sortable change list view objects not being grouped properly. +- Refactored the ORM calls to determine if an object is sortable, and what the next order should be, to return +scalar values and to not hydrate any objects whatsoever. This potentially decreases memory usage by exponential +factors. + +Features ============= Current --------- diff --git a/adminsortable/__init__.py b/adminsortable/__init__.py index e429140..44f81a6 100644 --- a/adminsortable/__init__.py +++ b/adminsortable/__init__.py @@ -1,4 +1,4 @@ -VERSION = (1, 1, 1, "f", 0) # following PEP 386 +VERSION = (1, 2, "f", 0) # following PEP 386 DEV_N = None diff --git a/adminsortable/admin.py b/adminsortable/admin.py index f80064a..8097647 100644 --- a/adminsortable/admin.py +++ b/adminsortable/admin.py @@ -53,18 +53,28 @@ class SortableAdmin(ModelAdmin): has_perm = request.user.has_perm(opts.app_label + '.' + opts.get_change_permission()) objects = self.model.objects.all() - """ - Determine if we need to regroup objects relative to a foreign key specified on the - model class that is extending Sortable. - """ + #Determine if we need to regroup objects relative to a foreign key specified on the + # model class that is extending Sortable. sortable_by = getattr(self.model, 'sortable_by', None) if sortable_by: - sortable_by_class, sortable_by_expression = sortable_by() + #backwards compatibility for < 1.1.1, where sortable_by was a classmethod instead of a property + try: + sortable_by_class, sortable_by_expression = sortable_by() + except TypeError, ValueError: + sortable_by_class = self.model.sortable_by + sortable_by_expression = sortable_by_class.__name__.lower() + sortable_by_class_display_name = sortable_by_class._meta.verbose_name_plural sortable_by_class_is_sortable = sortable_by_class.is_sortable() + + # Order the objects by the property they are sortable by, then by the order, otherwise the regroup + # template tag will not show the objects correctly as + # shown in https://docs.djangoproject.com/en/1.3/ref/templates/builtins/#regroup + objects = objects.order_by(sortable_by_expression, 'order') + else: - sortable_by_class = sortable_by_expression = sortable_by_class_display_name = \ - sortable_by_class_is_sortable = None + sortable_by_class = sortable_by_expression = sortable_by_class_display_name =\ + sortable_by_class_is_sortable = None try: verbose_name_plural = opts.verbose_name_plural.__unicode__() @@ -110,7 +120,6 @@ class SortableAdmin(ModelAdmin): This view sets the ordering of the objects for the model type and primary keys passed in. It must be an Ajax POST. """ - if request.is_ajax() and request.method == 'POST': try: indexes = map(str, request.POST.get('indexes', []).split(',')) @@ -131,7 +140,7 @@ class SortableAdmin(ModelAdmin): else: response = {'objects_sorted' : False} return HttpResponse(json.dumps(response, ensure_ascii=False), - mimetype='application/json') + mimetype='application/json') class SortableInlineBase(InlineModelAdmin): @@ -139,7 +148,8 @@ class SortableInlineBase(InlineModelAdmin): super(SortableInlineBase, self).__init__(*args, **kwargs) if not issubclass(self.model, Sortable): - raise Warning(u'Models that are specified in SortableTabluarInline and SortableStackedInline must inherit from Sortable') + raise Warning(u'Models that are specified in SortableTabluarInline and SortableStackedInline ' + 'must inherit from Sortable') self.is_sortable = self.model.is_sortable() diff --git a/adminsortable/models.py b/adminsortable/models.py index 0d74262..af299de 100644 --- a/adminsortable/models.py +++ b/adminsortable/models.py @@ -14,8 +14,12 @@ class Sortable(models.Model): `model_type_id` returns the ContentType.id for the Model that inherits Sortable `save` the override of save increments the last/highest value of order by 1 + + Override `sortable_by` method to make your model be sortable by a foreign key field. + Set `sortable_by` to the class specified in the foreign key relationship. """ order = models.PositiveIntegerField(editable=False, default=1, db_index=True) + sortable_by = None class Meta: abstract = True diff --git a/adminsortable/templates/adminsortable/change_list.html b/adminsortable/templates/adminsortable/change_list.html index f5e7562..9d85464 100644 --- a/adminsortable/templates/adminsortable/change_list.html +++ b/adminsortable/templates/adminsortable/change_list.html @@ -55,7 +55,7 @@ {% if objects %}
{% if group_expression %} - {% render_nested_sortable_objects objects group_expression %} + {% render_nested_sortable_objects objects group_expression %} {% else %} {% render_sortable_objects objects %} {% endif %} diff --git a/adminsortable/templates/adminsortable/shared/javascript_includes.html b/adminsortable/templates/adminsortable/shared/javascript_includes.html index 3aa396b..73c9590 100644 --- a/adminsortable/templates/adminsortable/shared/javascript_includes.html +++ b/adminsortable/templates/adminsortable/shared/javascript_includes.html @@ -5,6 +5,7 @@ + - \ No newline at end of file + diff --git a/adminsortable/templates/adminsortable/shared/nested_objects.html b/adminsortable/templates/adminsortable/shared/nested_objects.html index 850a37c..bcabb55 100644 --- a/adminsortable/templates/adminsortable/shared/nested_objects.html +++ b/adminsortable/templates/adminsortable/shared/nested_objects.html @@ -1,19 +1,18 @@ {% load django_template_additions adminsortable_tags %} {% dynamic_regroup objects by group_expression as regrouped_objects %} {% if regrouped_objects %} - - -{% endif %} \ No newline at end of file + +{% endif %} diff --git a/adminsortable/templatetags/django_template_additions.py b/adminsortable/templatetags/django_template_additions.py index 252b2e8..435ab90 100644 --- a/adminsortable/templatetags/django_template_additions.py +++ b/adminsortable/templatetags/django_template_additions.py @@ -25,11 +25,9 @@ class DynamicRegroupNode(template.Node): # List of dictionaries in the format: # {'grouper': 'key', 'list': [list of contents]}. - """ - Try to resolve the filter expression from the template context. - If the variable doesn't exist, accept the value that passed to the - template tag and convert it to a string - """ + #Try to resolve the filter expression from the template context. + #If the variable doesn't exist, accept the value that passed to the + #template tag and convert it to a string try: exp = self.expression.resolve(context) except template.VariableDoesNotExist: @@ -47,6 +45,15 @@ class DynamicRegroupNode(template.Node): @register.tag def dynamic_regroup(parser, token): + """ + Django expects the value of `expression` to be an attribute available on + your objects. The value you pass to the template tag gets converted into a + FilterExpression object from the literal. + + Sometimes we need the attribute to group on to be dynamic. So, instead + of converting the value to a FilterExpression here, we're going to pass the + value as-is and convert it in the Node. + """ firstbits = token.contents.split(None, 3) if len(firstbits) != 4: raise TemplateSyntaxError("'regroup' tag takes five arguments") @@ -58,20 +65,8 @@ def dynamic_regroup(parser, token): raise TemplateSyntaxError("next-to-last argument to 'regroup' tag must" " be 'as'") - """ - Django expects the value of `expression` to be an attribute available on - your objects. The value you pass to the template tag gets converted into a - FilterExpression object from the literal. - - Sometimes we need the attribute to group on to be dynamic. So, instead - of converting the value to a FilterExpression here, we're going to pass the - value as-is and convert it in the Node. - """ expression = lastbits_reversed[2][::-1] var_name = lastbits_reversed[0][::-1] - - """ - We also need to hand the parser to the node in order to convert the value - for `expression` to a FilterExpression. - """ + #We also need to hand the parser to the node in order to convert the value + #for `expression` to a FilterExpression. return DynamicRegroupNode(target, parser, expression, var_name) diff --git a/sample_project/app/admin.py b/sample_project/app/admin.py index a1eb445..f365aab 100644 --- a/sample_project/app/admin.py +++ b/sample_project/app/admin.py @@ -1,7 +1,7 @@ from django.contrib import admin from adminsortable.admin import SortableAdmin, SortableTabularInline, SortableStackedInline -from app.models import Category, Project, Credit, Note +from app.models import Category, Project, Credit, Note, Sample admin.site.register(Category, SortableAdmin) @@ -20,3 +20,4 @@ class ProjectAdmin(SortableAdmin): list_display = ['__unicode__', 'category'] admin.site.register(Project, ProjectAdmin) +admin.site.register(Sample, SortableAdmin) diff --git a/sample_project/app/migrations/0003_add_sample.py b/sample_project/app/migrations/0003_add_sample.py new file mode 100755 index 0000000..474c29e --- /dev/null +++ b/sample_project/app/migrations/0003_add_sample.py @@ -0,0 +1,68 @@ +# encoding: utf-8 +import datetime +from south.db import db +from south.v2 import SchemaMigration +from django.db import models + +class Migration(SchemaMigration): + + def forwards(self, orm): + + # Adding model 'Sample' + db.create_table('app_sample', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('order', self.gf('django.db.models.fields.PositiveIntegerField')(default=1, db_index=True)), + ('title', self.gf('django.db.models.fields.CharField')(max_length=50)), + ('category', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['app.Category'])), + ('description', self.gf('django.db.models.fields.TextField')()), + )) + db.send_create_signal('app', ['Sample']) + + + def backwards(self, orm): + + # Deleting model 'Sample' + db.delete_table('app_sample') + + + models = { + 'app.category': { + 'Meta': {'ordering': "['order']", 'object_name': 'Category'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}), + 'title': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'app.credit': { + 'Meta': {'ordering': "['order']", 'object_name': 'Credit'}, + 'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30'}), + 'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}), + 'project': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Project']"}) + }, + 'app.note': { + 'Meta': {'ordering': "['order']", 'object_name': 'Note'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}), + 'project': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Project']"}), + 'text': ('django.db.models.fields.CharField', [], {'max_length': '100'}) + }, + 'app.project': { + 'Meta': {'ordering': "['order']", 'object_name': 'Project'}, + 'category': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Category']"}), + 'description': ('django.db.models.fields.TextField', [], {}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}), + 'title': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + }, + 'app.sample': { + 'Meta': {'ordering': "['order']", 'object_name': 'Sample'}, + 'category': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Category']"}), + 'description': ('django.db.models.fields.TextField', [], {}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}), + 'title': ('django.db.models.fields.CharField', [], {'max_length': '50'}) + } + } + + complete_apps = ['app'] diff --git a/sample_project/app/models.py b/sample_project/app/models.py index caba787..9b58536 100644 --- a/sample_project/app/models.py +++ b/sample_project/app/models.py @@ -29,6 +29,8 @@ class Project(SimpleModel, Sortable): class Meta(Sortable.Meta): pass + #deprecated: shown for backward compatibility only. Reference class "Sample" for proper + # designation of `sortable_by` as a property @classmethod def sortable_by(cls): return Category, 'category' @@ -37,6 +39,19 @@ class Project(SimpleModel, Sortable): description = models.TextField() +#a model that is sortable relative to a foreign key that is also sortable +class Sample(SimpleModel, Sortable): + class Meta(Sortable.Meta): + ordering = Sortable.Meta.ordering + ['category'] + + category = models.ForeignKey(Category) + description = models.TextField() + + #field to define which foreign key the model is sortable by. + #works with versions > 1.1.1 + sortable_by = Category + + #registered as a tabular inline on project class Credit(Sortable): class Meta(Sortable.Meta): diff --git a/sample_project/app/tests.py b/sample_project/app/tests.py index 3eb1677..a086099 100644 --- a/sample_project/app/tests.py +++ b/sample_project/app/tests.py @@ -1,12 +1,9 @@ import httplib import json -from django.contrib.auth.forms import authenticate, UserCreationForm from django.contrib.auth.models import User from django.core.urlresolvers import reverse -from django.middleware import csrf from django.db import models -from django.template.defaultfilters import urlencode from django.test import TestCase from django.test.client import Client, RequestFactory diff --git a/sample_project/settings.py b/sample_project/settings.py index c16655e..0309bf8 100644 --- a/sample_project/settings.py +++ b/sample_project/settings.py @@ -102,7 +102,6 @@ MIDDLEWARE_CLASSES = ( 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', - 'debug_toolbar.middleware.DebugToolbarMiddleware', ) TEMPLATE_CONTEXT_PROCESSORS = (