From 8b3c05afa55f963d6e966707737cbc639f697239 Mon Sep 17 00:00:00 2001
From: Adrian Holovaty <adrian@holovaty.com>
Date: Fri, 22 Jul 2005 13:02:27 +0000
Subject: [PATCH] Fixed #146 -- Changed order_by and ordering parameters to be
 less verbose. The old syntax is still supported but will not be supported by
 first release.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@292 bcc190cf-cafb-0310-a4f2-bffc1f526a37
---
 django/contrib/comments/models/comments.py    |  4 +-
 .../contrib/comments/templatetags/comments.py | 50 ++++++++--------
 django/core/meta.py                           | 60 +++++++++++++++----
 django/models/auth.py                         |  8 +--
 django/models/core.py                         | 10 ++--
 django/views/admin/main.py                    | 15 +++--
 django/views/generic/date_based.py            |  2 +-
 7 files changed, 93 insertions(+), 56 deletions(-)

diff --git a/django/contrib/comments/models/comments.py b/django/contrib/comments/models/comments.py
index 18a6f2cf3f..83b60f88c3 100644
--- a/django/contrib/comments/models/comments.py
+++ b/django/contrib/comments/models/comments.py
@@ -42,7 +42,7 @@ class Comment(meta.Model):
         'RATINGS_OPTIONAL': 'ra',
         'IS_PUBLIC': 'ip',
     }
-    ordering = (('submit_date', 'DESC'),)
+    ordering = ('-submit_date',)
     admin = meta.Admin(
         fields = (
             (None, {'fields': ('content_type_id', 'object_id', 'site_id')}),
@@ -170,7 +170,7 @@ class FreeComment(meta.Model):
         meta.BooleanField('approved', 'approved by staff'),
         meta.ForeignKey(core.Site),
     )
-    ordering = (('submit_date', 'DESC'),)
+    ordering = ('-submit_date',)
     admin = meta.Admin(
         fields = (
             (None, {'fields': ('content_type_id', 'object_id', 'site_id')}),
diff --git a/django/contrib/comments/templatetags/comments.py b/django/contrib/comments/templatetags/comments.py
index d9fe0e3a39..99bf50fd12 100644
--- a/django/contrib/comments/templatetags/comments.py
+++ b/django/contrib/comments/templatetags/comments.py
@@ -148,7 +148,7 @@ class CommentListNode(template.Node):
             'content_type__python_module_name__exact': self.module,
             'site_id__exact': SITE_ID,
             'select_related': True,
-            'order_by': (('submit_date', self.ordering),),
+            'order_by': (self.ordering + 'submit_date',),
         }
         if not self.free and COMMENTS_BANNED_USERS_GROUP:
             kwargs['select'] = {'is_hidden': 'user_id IN (SELECT user_id FROM auth_users_groups WHERE group_id = %s)' % COMMENTS_BANNED_USERS_GROUP}
@@ -170,16 +170,16 @@ class CommentListNode(template.Node):
 
 class DoCommentForm:
     """
-    Displays a comment form for the given params. 
-    
+    Displays a comment form for the given params.
+
     Syntax::
-    
+
         {% comment_form for [pkg].[py_module_name] [context_var_containing_obj_id] with [list of options] %}
-    
+
     Example usage::
-    
+
         {% comment_form for lcom.eventtimes event.id with is_public yes photos_optional thumbs,200,400 ratings_optional scale:1-5|first_option|second_option %}
-    
+
     ``[context_var_containing_obj_id]`` can be a hard-coded integer or a variable containing the ID.
     """
     def __init__(self, free, tag_name):
@@ -246,18 +246,18 @@ class DoCommentCount:
     """
     Gets comment count for the given params and populates the template context
     with a variable containing that value, whose name is defined by the 'as'
-    clause. 
-    
+    clause.
+
     Syntax::
-        
+
         {% get_comment_count for [pkg].[py_module_name] [context_var_containing_obj_id] as [varname]  %}
-    
+
     Example usage::
-    
+
         {% get_comment_count for lcom.eventtimes event.id as comment_count %}
-        
+
     Note: ``[context_var_containing_obj_id]`` can also be a hard-coded integer, like this::
-    
+
         {% get_comment_count for lcom.eventtimes 23 as comment_count %}
     """
     def __init__(self, free, tag_name):
@@ -297,22 +297,22 @@ class DoGetCommentList:
     Gets comments for the given params and populates the template context with a
     special comment_package variable, whose name is defined by the ``as``
     clause.
-    
+
     Syntax::
-    
+
         {% get_comment_list for [pkg].[py_module_name] [context_var_containing_obj_id] as [varname] (reversed) %}
-    
+
     Example usage::
-    
+
         {% get_comment_list for lcom.eventtimes event.id as comment_list %}
-    
+
     Note: ``[context_var_containing_obj_id]`` can also be a hard-coded integer, like this::
-    
+
         {% get_comment_list for lcom.eventtimes 23 as comment_list %}
-        
-    To get a list of comments in reverse order -- that is, most recent first -- 
+
+    To get a list of comments in reverse order -- that is, most recent first --
     pass ``reversed`` as the last param::
-    
+
         {% get_comment_list for lcom.eventtimes event.id as comment_list reversed %}
     """
     def __init__(self, free, tag_name):
@@ -348,9 +348,9 @@ class DoGetCommentList:
         if len(tokens) == 7:
             if tokens[6] != 'reversed':
                 raise template.TemplateSyntaxError, "Final argument in '%s' must be 'reversed' if given" % self.tag_name
-            ordering = "DESC"
+            ordering = "-"
         else:
-            ordering = "ASC"
+            ordering = ""
         return CommentListNode(package, module, var_name, obj_id, tokens[5], self.free, ordering)
 
 # registration comments
diff --git a/django/core/meta.py b/django/core/meta.py
index 5c8d070f40..c328fa0823 100644
--- a/django/core/meta.py
+++ b/django/core/meta.py
@@ -52,6 +52,36 @@ prep_for_like_query = lambda x: str(x).replace("%", "\%").replace("_", "\_")
 # returns the <ul> class for a given radio_admin value
 get_ul_class = lambda x: 'radiolist%s' % ((x == HORIZONTAL) and ' inline' or '')
 
+# Django currently supports two forms of ordering.
+# Form 1 (deprecated) example:
+#     order_by=(('pub_date', 'DESC'), ('headline', 'ASC'), (None, 'RANDOM'))
+# Form 2 (new-style) example:
+#     order_by=('-pub_date', 'headline', '?')
+# Form 1 is deprecated and will no longer be supported for Django's first
+# official release. The following code converts from Form 1 to Form 2.
+
+LEGACY_ORDERING_MAPPING = {'ASC': '_', 'DESC': '-_', 'RANDOM': '?'}
+
+def handle_legacy_orderlist(order_list):
+    if not order_list or isinstance(order_list[0], basestring):
+        return order_list
+    else:
+#         import warnings
+        new_order_list = [LEGACY_ORDERING_MAPPING[j.upper()].replace('_', str(i)) for i, j in order_list]
+#         warnings.warn("%r ordering syntax is deprecated. Use %r instead." % (order_list, new_order_list), DeprecationWarning)
+        return new_order_list
+
+def orderlist2sql(order_list, prefix=''):
+    output = []
+    for f in handle_legacy_orderlist(order_list):
+        if f.startswith('-'):
+            output.append('%s%s DESC' % (prefix, f[1:]))
+        elif f == '?':
+            output.append('RANDOM()')
+        else:
+            output.append('%s%s ASC' % (prefix, f))
+    return ', '.join(output)
+
 def curry(*args, **kwargs):
     def _curried(*moreargs, **morekwargs):
         return args[0](*(args[1:]+moreargs), **dict(kwargs.items() + morekwargs.items()))
@@ -175,7 +205,7 @@ class Options:
         self.get_latest_by = get_latest_by
         if order_with_respect_to:
             self.order_with_respect_to = self.get_field(order_with_respect_to)
-            self.ordering = (('_order', 'ASC'),)
+            self.ordering = ('_order',)
         else:
             self.order_with_respect_to = None
         self.module_constants = module_constants or {}
@@ -231,7 +261,7 @@ class Options:
         "Returns the full 'ORDER BY' clause for this object, according to self.ordering."
         if not self.ordering: return ''
         pre = table_prefix and (table_prefix + '.') or ''
-        return 'ORDER BY ' + ','.join(['%s%s %s' % (pre, f, order) for f, order in self.ordering])
+        return 'ORDER BY ' + orderlist2sql(self.ordering, pre)
 
     def get_add_permission(self):
         return 'add_%s' % self.object_name.lower()
@@ -770,7 +800,7 @@ def method_delete(opts, self):
 
 def method_get_next_in_order(opts, order_field, self):
     if not hasattr(self, '_next_in_order_cache'):
-        self._next_in_order_cache = opts.get_model_module().get_object(order_by=(('_order', 'ASC'),),
+        self._next_in_order_cache = opts.get_model_module().get_object(order_by=('_order',),
             where=['_order > (SELECT _order FROM %s WHERE %s=%%s)' % (opts.db_table, opts.pk.name),
                 '%s=%%s' % order_field.name], limit=1,
             params=[getattr(self, opts.pk.name), getattr(self, order_field.name)])
@@ -778,7 +808,7 @@ def method_get_next_in_order(opts, order_field, self):
 
 def method_get_previous_in_order(opts, order_field, self):
     if not hasattr(self, '_previous_in_order_cache'):
-        self._previous_in_order_cache = opts.get_model_module().get_object(order_by=(('_order', 'DESC'),),
+        self._previous_in_order_cache = opts.get_model_module().get_object(order_by=('-_order',),
             where=['_order < (SELECT _order FROM %s WHERE %s=%%s)' % (opts.db_table, opts.pk.name),
                 '%s=%%s' % order_field.name], limit=1,
             params=[getattr(self, opts.pk.name), getattr(self, order_field.name)])
@@ -908,7 +938,7 @@ def method_get_order(ordered_obj, self):
 def method_get_next_or_previous(get_object_func, field, is_next, self, **kwargs):
     kwargs.setdefault('where', []).append('%s %s %%s' % (field.name, (is_next and '>' or '<')))
     kwargs.setdefault('params', []).append(str(getattr(self, field.name)))
-    kwargs['order_by'] = ((field.name, (is_next and 'ASC' or 'DESC')),)
+    kwargs['order_by'] = [(not is_next and '-' or '') + field.name]
     kwargs['limit'] = 1
     return get_object_func(**kwargs)
 
@@ -1216,16 +1246,20 @@ def function_get_sql_clause(opts, **kwargs):
 
     # ORDER BY clause
     order_by = []
-    for i, j in kwargs.get('order_by', opts.ordering):
-        if j == "RANDOM":
-            order_by.append("RANDOM()")
+    for f in handle_legacy_orderlist(kwargs.get('order_by', opts.ordering)):
+        if f == '?': # Special case.
+            order_by.append('RANDOM()')
         else:
-            # Append the database table as a column prefix if it wasn't given,
+            # Use the database table as a column prefix if it wasn't given,
             # and if the requested column isn't a custom SELECT.
-            if "." not in i and i not in [k[0] for k in kwargs.get('select', [])]:
-                order_by.append("%s.%s %s" % (opts.db_table, i, j))
+            if "." not in f and f not in [k[0] for k in kwargs.get('select', [])]:
+                table_prefix = opts.db_table + '.'
             else:
-                order_by.append("%s %s" % (i, j))
+                table_prefix = ''
+            if f.startswith('-'):
+                order_by.append('%s%s DESC' % (table_prefix, f[1:]))
+            else:
+                order_by.append('%s%s ASC' % (table_prefix, f))
     order_by = ", ".join(order_by)
 
     # LIMIT and OFFSET clauses
@@ -1246,7 +1280,7 @@ def function_get_in_bulk(opts, klass, *args, **kwargs):
     return dict([(o.id, o) for o in obj_list])
 
 def function_get_latest(opts, klass, does_not_exist_exception, **kwargs):
-    kwargs['order_by'] = ((opts.get_latest_by, "DESC"),)
+    kwargs['order_by'] = ('-' + opts.get_latest_by,)
     kwargs['limit'] = 1
     return function_get_object(opts, klass, does_not_exist_exception, **kwargs)
 
diff --git a/django/models/auth.py b/django/models/auth.py
index 516ec112df..d0d2bf5002 100644
--- a/django/models/auth.py
+++ b/django/models/auth.py
@@ -8,7 +8,7 @@ class Permission(meta.Model):
         meta.CharField('codename', 'code name', maxlength=100),
     )
     unique_together = (('package', 'codename'),)
-    ordering = (('package', 'ASC'), ('codename', 'ASC'))
+    ordering = ('package', 'codename')
 
     def __repr__(self):
         return "%s | %s" % (self.package, self.name)
@@ -18,7 +18,7 @@ class Group(meta.Model):
         meta.CharField('name', 'name', maxlength=80, unique=True),
         meta.ManyToManyField(Permission, blank=True, filter_interface=meta.HORIZONTAL),
     )
-    ordering = (('name', 'ASC'),)
+    ordering = ('name',)
     admin = meta.Admin(
         search_fields = ('name',),
     )
@@ -44,7 +44,7 @@ class User(meta.Model):
             help_text="In addition to the permissions manually assigned, this user will also get all permissions granted to each group he/she is in."),
         meta.ManyToManyField(Permission, name='user_permissions', blank=True, filter_interface=meta.HORIZONTAL),
     )
-    ordering = (('username', 'ASC'),)
+    ordering = ('username',)
     exceptions = ('SiteProfileNotAvailable',)
     admin = meta.Admin(
         fields = (
@@ -253,7 +253,7 @@ class LogEntry(meta.Model):
         meta.PositiveSmallIntegerField('action_flag', 'action flag'),
         meta.TextField('change_message', 'change message', blank=True),
     )
-    ordering = (('action_time', 'DESC'),)
+    ordering = ('-action_time',)
     module_constants = {
         'ADDITION': 1,
         'CHANGE': 2,
diff --git a/django/models/core.py b/django/models/core.py
index bdf4567e6a..bf74610db2 100644
--- a/django/models/core.py
+++ b/django/models/core.py
@@ -6,7 +6,7 @@ class Site(meta.Model):
         meta.CharField('domain', 'domain name', maxlength=100),
         meta.CharField('name', 'display name', maxlength=50),
     )
-    ordering = (('domain', 'ASC'),)
+    ordering = ('domain',)
 
     def __repr__(self):
         return self.domain
@@ -22,7 +22,7 @@ class Package(meta.Model):
         meta.CharField('label', 'label', maxlength=20, primary_key=True),
         meta.CharField('name', 'name', maxlength=30, unique=True),
     )
-    ordering = (('name', 'ASC'),)
+    ordering = ('name',)
 
     def __repr__(self):
         return self.name
@@ -34,7 +34,7 @@ class ContentType(meta.Model):
         meta.ForeignKey(Package, name='package'),
         meta.CharField('python_module_name', 'Python module name', maxlength=50),
     )
-    ordering = (('package', 'ASC'), ('name', 'ASC'),)
+    ordering = ('package', 'name')
     unique_together = (('package', 'python_module_name'),)
 
     def __repr__(self):
@@ -63,7 +63,7 @@ class Redirect(meta.Model):
             help_text="This can be either an absolute path (as above) or a full URL starting with 'http://'."),
     )
     unique_together=(('site_id', 'old_path'),)
-    ordering = (('old_path', 'ASC'),)
+    ordering = ('old_path',)
     admin = meta.Admin(
         list_display = ('__repr__',),
         list_filter = ('site_id',),
@@ -87,7 +87,7 @@ class FlatFile(meta.Model):
             help_text="If this is checked, only logged-in users will be able to view the page."),
         meta.ManyToManyField(Site),
     )
-    ordering = (('url', 'ASC'),)
+    ordering = ('url',)
     admin = meta.Admin(
         fields = (
             (None, {'fields': ('url', 'title', 'content', 'sites')}),
diff --git a/django/views/admin/main.py b/django/views/admin/main.py
index b5dfb1912d..45d044727d 100644
--- a/django/views/admin/main.py
+++ b/django/views/admin/main.py
@@ -94,12 +94,15 @@ def change_list(request, app_label, module_name):
     # then check the object's default ordering. If neither of those exist,
     # order descending by ID by default. Finally, look for manually-specified
     # ordering from the query string.
-    if lookup_opts.admin.ordering is not None:
-        order_field, order_type = lookup_opts.admin.ordering
-    elif lookup_opts.ordering:
-        order_field, order_type = lookup_opts.ordering[0]
+    ordering = lookup_opts.admin.ordering or lookup_opts.ordering or ('-' + lookup_opts.pk.name)
+
+    # Normalize it to new-style ordering.
+    ordering = meta.handle_legacy_orderlist(ordering)
+
+    if ordering[0].startswith('-'):
+        order_field, order_type = ordering[0][1:], 'DESC'
     else:
-        order_field, order_type = lookup_opts.pk.name, 'DESC'
+        order_field, order_type = ordering[0], 'ASC'
     if params.has_key(ORDER_VAR):
         try:
             try:
@@ -1071,7 +1074,7 @@ def delete_stage(request, app_label, module_name, object_id):
 def history(request, app_label, module_name, object_id):
     mod, opts = _get_mod_opts(app_label, module_name)
     action_list = log.get_list(object_id__exact=object_id, content_type_id__exact=opts.get_content_type_id(),
-        order_by=(("action_time", "ASC"),), select_related=True)
+        order_by=("action_time",), select_related=True)
     # If no history was found, see whether this object even exists.
     try:
         obj = mod.get_object(id__exact=object_id)
diff --git a/django/views/generic/date_based.py b/django/views/generic/date_based.py
index e598f13694..c204ee035a 100644
--- a/django/views/generic/date_based.py
+++ b/django/views/generic/date_based.py
@@ -27,7 +27,7 @@ def archive_index(request, app_label, module_name, date_field, num_latest=15, te
     if num_latest:
         lookup_kwargs.update({
             'limit': num_latest,
-            'order_by': ((date_field, 'DESC'),),
+            'order_by': ('-' + date_field,),
         })
         latest = mod.get_list(**lookup_kwargs)
     else: