From 39bc7eb75de9fe6a1aed20767da435b5f3e1c95a Mon Sep 17 00:00:00 2001
From: Irina Peshinskaya <ipeshinskaya@sipwise.com>
Date: Thu, 13 Dec 2018 00:32:01 +0100
Subject: [PATCH] TT#48198 Fix order_by functionality in api

- All item_rs modifications should be done before we get rows, so apply order_by before pager
- We can't distinguish if subscriberregistrations really has  column or not if we return true for all columns.
We will use has_column only in cases when it returns something really meaningful.
- Subscriberregistrations can't order by nat and subscriber_id

Change-Id: I04b7bb719ee058590a7705c6411cb08bcfb15387
---
 .../Controller/API/SubscriberRegistrations.pm |  2 -
 lib/NGCP/Panel/Role/API.pm                    | 57 ++++++++++---------
 .../Panel/Utils/RedisLocationResultSource.pm  |  5 --
 3 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm b/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm
index 15a974902b..475d02e5f3 100644
--- a/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm
+++ b/lib/NGCP/Panel/Controller/API/SubscriberRegistrations.pm
@@ -60,10 +60,8 @@ sub order_by_cols {
         'contact' => 'contact',
         'expires' => 'expires',
         'id'      => 'id',
-        'nat'     => 'nat',
         'path'    => 'path',
         'q'       => 'q',
-        'subscriber_id' => 'subscriber_id',
         'user_agent'    => 'user_agent',
     };
     return $cols;
diff --git a/lib/NGCP/Panel/Role/API.pm b/lib/NGCP/Panel/Role/API.pm
index eba0c3f1d6..699baee68e 100644
--- a/lib/NGCP/Panel/Role/API.pm
+++ b/lib/NGCP/Panel/Role/API.pm
@@ -725,9 +725,37 @@ sub paginate_order_collection_rs {
     my($page,$rows,$order_by,$direction) = @$params{qw/page rows order_by direction/};
 
     my $result_class = $item_rs->result_class();
+    my $items = [];
 
+    if ($order_by) {
+        my $explicit_order_col_spec;
+        if ($self->can('order_by_cols')) {
+            my($explicit_order_cols,$explicit_order_cols_params) = $self->order_by_cols($c);
+            $explicit_order_col_spec = $explicit_order_cols->{$order_by};
+            $explicit_order_cols_params //= {};
+            if ( exists $explicit_order_cols_params->{$order_by}->{join} ) {
+                $item_rs = $item_rs->search(undef, {
+                    join => $explicit_order_cols_params->{$order_by}->{join},
+                });
+            }
+        }
+        if ($explicit_order_col_spec || 
+            ( $item_rs->result_source->can('has_column') && $item_rs->result_source->has_column($order_by) )) {
+            my $col = $explicit_order_col_spec || $item_rs->current_source_alias . '.' . $order_by;
+            if (lc($direction) eq 'desc') {
+                $item_rs = $item_rs->search(undef, {
+                    order_by => {-desc => $col},
+                });
+                $c->log->debug("ordering by $col DESC");
+            } else {
+                $item_rs = $item_rs->search(undef, {
+                    order_by => "$col",
+                });
+                $c->log->debug("ordering by $col");
+            }
+        }
+    }
     my $total_count;
-    my $items = [];
     my $no_count = $self->dont_count_collection_total($c);
     if ( !$no_count ) {
         $total_count = int($item_rs->count);
@@ -755,33 +783,6 @@ sub paginate_order_collection_rs {
         $self->define_collection_infinite_pager($c, $item_rs_count, $rows, $no_count);
     }
 
-    if ($order_by) {
-        my $explicit_order_col_spec;
-        if ($self->can('order_by_cols')) {
-            my($explicit_order_cols,$explicit_order_cols_params) = $self->order_by_cols($c);
-            $explicit_order_col_spec = $explicit_order_cols->{$order_by};
-            $explicit_order_cols_params //= {};
-            if ( exists $explicit_order_cols_params->{$order_by}->{join} ) {
-                $item_rs = $item_rs->search(undef, {
-                    join => $explicit_order_cols_params->{$order_by}->{join},
-                });
-            }
-        }
-        if ($explicit_order_col_spec || $item_rs->result_source->has_column($order_by)) {
-            my $col = $explicit_order_col_spec || $item_rs->current_source_alias . '.' . $order_by;
-            if (lc($direction) eq 'desc') {
-                $item_rs = $item_rs->search(undef, {
-                    order_by => {-desc => $col},
-                });
-                $c->log->debug("ordering by $col DESC");
-            } else {
-                $item_rs = $item_rs->search(undef, {
-                    order_by => "$col",
-                });
-                $c->log->debug("ordering by $col");
-            }
-        }
-    }
     my $result_class_after = $item_rs->result_class();
     if($result_class ne $result_class_after){
         $item_rs->result_class($result_class);
diff --git a/lib/NGCP/Panel/Utils/RedisLocationResultSource.pm b/lib/NGCP/Panel/Utils/RedisLocationResultSource.pm
index 212de85a1a..d9f6af4587 100644
--- a/lib/NGCP/Panel/Utils/RedisLocationResultSource.pm
+++ b/lib/NGCP/Panel/Utils/RedisLocationResultSource.pm
@@ -27,10 +27,5 @@ sub columns {
     
 }
 
-sub has_column {
-    my ($self, $column) = @_;
-    return 1;
-}
-
 
 1;