diff --git a/README b/README index f132a19..1f6f71a 100755 --- 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 100755 --- 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 b389546..8097647 100755 --- a/adminsortable/admin.py +++ b/adminsortable/admin.py @@ -60,16 +60,21 @@ class SortableAdmin(ModelAdmin): #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 ValueError: + except TypeError, ValueError: sortable_by_class = self.model.sortable_by sortable_by_expression = sortable_by_class.__name__.lower() - print sortable_by_expression 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__() @@ -115,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(',')) @@ -136,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): @@ -144,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/templates/adminsortable/shared/javascript_includes.html b/adminsortable/templates/adminsortable/shared/javascript_includes.html index 3aa396b..73c9590 100755 --- 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 100755 --- 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 4de7630..435ab90 100755 --- 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] - print expression - """ - 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/adminsortable.sqlite b/sample_project/adminsortable.sqlite index df95757..7df8bd7 100755 Binary files a/sample_project/adminsortable.sqlite and b/sample_project/adminsortable.sqlite differ diff --git a/sample_project/app/migrations/0003_add_sample.py b/sample_project/app/migrations/0003_add_sample.py old mode 100644 new mode 100755 index f683706..474c29e --- a/sample_project/app/migrations/0003_add_sample.py +++ b/sample_project/app/migrations/0003_add_sample.py @@ -1,68 +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'] +# 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 2e8aab0..9b58536 100755 --- a/sample_project/app/models.py +++ b/sample_project/app/models.py @@ -29,20 +29,20 @@ class Project(SimpleModel, Sortable): class Meta(Sortable.Meta): pass -# @classmethod -# def sortable_by(cls): -# return Category, 'category' + #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' category = models.ForeignKey(Category) description = models.TextField() - sortable_by = Category - #a model that is sortable relative to a foreign key that is also sortable class Sample(SimpleModel, Sortable): class Meta(Sortable.Meta): - pass + ordering = Sortable.Meta.ordering + ['category'] category = models.ForeignKey(Category) description = models.TextField() diff --git a/sample_project/app/tests.py b/sample_project/app/tests.py index 13a59ad..a086099 100755 --- a/sample_project/app/tests.py +++ b/sample_project/app/tests.py @@ -80,9 +80,6 @@ class SortableTestCase(TestCase): def get_category_indexes(self, *categories): return {'indexes' : ','.join([str(c.id) for c in categories])} - def test_sortable_by_backwards_compatibility(self): - pass - def test_adminsortable_changelist_templates(self): logged_in = self.client.login(username=self.user.username, password=self.user_raw_password) self.assertTrue(logged_in, 'User is not logged in')