Incremented version to 1.2.

Refactored ORM calls to properly order objects by the sortable_by property to ensure objects are grouped correctly in the sortable change list template after being passed through dynamic_regroup.
Fixed missing import for jquery.effects.core, again.
Refactored sortable_by classmethod into a property.

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
+specify a property: ``sortable_by`` that is equal to the class defined as your ForeignKey field.
-name of the foreign key property as defined on your model, as a string.
+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
+        sortable_by = Category
-        def sortable_by(cls):
-            return Category, '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
 ---------

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
 
 

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 = sortable_by_expression = sortable_by_class_display_name =\
-                sortable_by_class_is_sortable = None
+            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()
 

adminsortable/templates/adminsortable/shared/javascript_includes.html

@@ -5,6 +5,7 @@
 <script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/jquery.ui.draggable.js"></script>
 <script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/jquery.ui.droppable.js"></script>
 <script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/jquery.ui.sortable.js"></script>
+<script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/jquery.effects.core.js"></script>
 <script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/jquery.effects.highlight.js"></script>
 <script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/admin.sortable.stacked.inlines.js"></script>
-<script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/admin.sortable.js"></script>
+<script type="text/javascript" src="{{ STATIC_URL }}adminsortable/js/admin.sortable.js"></script>

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 %}
-
+    <ul {% if sortable_by_class_is_sortable %}class="sortable"{% endif %}>
-	<ul {% if sortable_by_class_is_sortable %}class="sortable"{% endif %}>
+        {% for regrouped_object in regrouped_objects %}
-	{% for regrouped_object in regrouped_objects %}
+            <li>
-		<li>
+                {% with object=regrouped_object.grouper %}
-			{% with object=regrouped_object.grouper %}
+                    {% render_object_rep object %}
-			    {% render_object_rep object %}
+                {% endwith %}
-			{% endwith %}
+                {% if regrouped_object.list %}
-			{% if regrouped_object.list %}
+                    <ul {% if regrouped_object.grouper.is_sortable %}class="sortable"{% endif %}>
-				<ul {% if regrouped_object.grouper.is_sortable %}class="sortable"{% endif %}>
+                        {% render_list_items regrouped_object.list %}
-					{% render_list_items regrouped_object.list %}
+                    </ul>
-				</ul>
+                {% endif %}
-			{% endif %}
+            </li>
-		</li>
+        {% endfor %}
-	{% endfor %}
+    </ul>
-	</ul>
+{% endif %}
-{% endif %}

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.
-        Try to resolve the filter expression from the template context.
+        #If the variable doesn't exist, accept the value that passed to the
-        If the variable doesn't exist, accept the value that passed to the
+        #template tag and convert it to a string
-        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)

sample_project/app/migrations/0003_add_sample.py

@@ -1,68 +1,68 @@
-# encoding: utf-8
+# encoding: utf-8
-import datetime
+import datetime
-from south.db import db
+from south.db import db
-from south.v2 import SchemaMigration
+from south.v2 import SchemaMigration
-from django.db import models
+from django.db import models
-
+
-class Migration(SchemaMigration):
+class Migration(SchemaMigration):
-
+
-    def forwards(self, orm):
+    def forwards(self, orm):
-        
+        
-        # Adding model 'Sample'
+        # Adding model 'Sample'
-        db.create_table('app_sample', (
+        db.create_table('app_sample', (
-            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
-            ('order', self.gf('django.db.models.fields.PositiveIntegerField')(default=1, db_index=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)),
+            ('title', self.gf('django.db.models.fields.CharField')(max_length=50)),
-            ('category', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['app.Category'])),
+            ('category', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['app.Category'])),
-            ('description', self.gf('django.db.models.fields.TextField')()),
+            ('description', self.gf('django.db.models.fields.TextField')()),
-        ))
+        ))
-        db.send_create_signal('app', ['Sample'])
+        db.send_create_signal('app', ['Sample'])
-
+
-
+
-    def backwards(self, orm):
+    def backwards(self, orm):
-        
+        
-        # Deleting model 'Sample'
+        # Deleting model 'Sample'
-        db.delete_table('app_sample')
+        db.delete_table('app_sample')
-
+
-
+
-    models = {
+    models = {
-        'app.category': {
+        'app.category': {
-            'Meta': {'ordering': "['order']", 'object_name': 'Category'},
+            'Meta': {'ordering': "['order']", 'object_name': 'Category'},
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
+            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
-            'title': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '50'})
-        },
+        },
-        'app.credit': {
+        'app.credit': {
-            'Meta': {'ordering': "['order']", 'object_name': 'Credit'},
+            'Meta': {'ordering': "['order']", 'object_name': 'Credit'},
-            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30'}),
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30'}),
-            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
+            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
-            'project': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Project']"})
+            'project': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Project']"})
-        },
+        },
-        'app.note': {
+        'app.note': {
-            'Meta': {'ordering': "['order']", 'object_name': 'Note'},
+            'Meta': {'ordering': "['order']", 'object_name': 'Note'},
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
+            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
-            'project': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Project']"}),
+            'project': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Project']"}),
-            'text': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+            'text': ('django.db.models.fields.CharField', [], {'max_length': '100'})
-        },
+        },
-        'app.project': {
+        'app.project': {
-            'Meta': {'ordering': "['order']", 'object_name': 'Project'},
+            'Meta': {'ordering': "['order']", 'object_name': 'Project'},
-            'category': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Category']"}),
+            'category': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Category']"}),
-            'description': ('django.db.models.fields.TextField', [], {}),
+            'description': ('django.db.models.fields.TextField', [], {}),
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
+            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
-            'title': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '50'})
-        },
+        },
-        'app.sample': {
+        'app.sample': {
-            'Meta': {'ordering': "['order']", 'object_name': 'Sample'},
+            'Meta': {'ordering': "['order']", 'object_name': 'Sample'},
-            'category': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Category']"}),
+            'category': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['app.Category']"}),
-            'description': ('django.db.models.fields.TextField', [], {}),
+            'description': ('django.db.models.fields.TextField', [], {}),
-            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
-            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
+            'order': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1', 'db_index': 'True'}),
-            'title': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+            'title': ('django.db.models.fields.CharField', [], {'max_length': '50'})
-        }
+        }
-    }
+    }
-
+
-    complete_apps = ['app']
+    complete_apps = ['app']

sample_project/app/models.py

@@ -29,20 +29,20 @@ class Project(SimpleModel, Sortable):
     class Meta(Sortable.Meta):
         pass
 
-#    @classmethod
+    #deprecated: shown for backward compatibility only. Reference class "Sample" for proper
-#    def sortable_by(cls):
+    # designation of `sortable_by` as a property
-#        return Category, 'category'
+    @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()

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')