#791 [frontend] fix for api3 pagination ordering
Merged 5 months ago by praiskup. Opened 5 months ago by praiskup.
copr/ praiskup/copr paginator-order  into  master

@@ -117,7 +117,6 @@ 

      LIMIT = None

      OFFSET = 0

      ORDER = "id"

-     ORDER_TYPE = "ASC"

  

      def __init__(self, query, model, limit=None, offset=None, order=None, order_type=None, **kwargs):

          self.query = query

@@ -125,15 +124,26 @@ 

          self.limit = limit or self.LIMIT

          self.offset = offset or self.OFFSET

          self.order = order or self.ORDER

-         self.order_type = order_type or self.ORDER_TYPE

+         self.order_type = order_type

+         if not self.order_type:

+             # desc/asc unspecified, use some guessed defaults

+             if self.order == 'id':

+                 self.order_type = 'DESC'

+             if self.order == 'name':

What about having just else here? The only exception that comes to my mind, that we want also have DESC, are dates.

Maybe, I don't have strong preference (I intentionally tried to let the default unspecified, since it looked like more natural; and the default is ASC anyways).

+                 self.order_type = 'ASC'

+ 

  

      def get(self):

          if not hasattr(self.model, self.order):

              raise CoprHttpException("Can order by: {}".format(self.order))

-         order_col = self.order

-         order_fun = sqlalchemy.desc if self.order_type == "DESC" else sqlalchemy.asc

-         return self.query.order_by(None).limit(None).offset(None)\

-             .order_by(order_fun(order_col)).limit(self.limit).offset(self.offset)

+ 

+         order_fun = (lambda x: x)

+         if self.order_type == 'ASC':

+             order_fun = sqlalchemy.asc

+         elif self.order_type == 'DESC':

+             order_fun = sqlalchemy.desc

+ 

+         return self.query.order_by(order_fun(self.order)).limit(self.limit).offset(self.offset)

  

      @property

      def meta(self):

Make sure that we don't override explicit ORDER BY request from
Paginator caller (reverts 3e73f63), and that we
use sane defaults:
- by default order by field: model.id
- if ORDER BY id => use DESC by default
- if ORDER BY name => use ASC by default
- otherwise keep system default ASC/DESC

Fixes: rhbz#1717506
Fixes: PR#791

rebased onto f1ef2f7a1f177080a1852bbafd7049efd873e9b6

5 months ago

[copr-build] mirror sync issue

What about having just else here? The only exception that comes to my mind, that we want also have DESC, are dates.

Looks good, thank you. +1

Maybe, I don't have strong preference (I intentionally tried to let the default unspecified, since it looked like more natural; and the default is ASC anyways).

No strictly required changes, so I'm merging. We can tweak the defaults anytime later.

rebased onto 51681b4

5 months ago

Pull-Request has been merged by praiskup

5 months ago