MT#16171 useless joins for source_subs = OR dest_subs =

+subscriber instead of source_subscriber typo fixed
+rudimentary test case to see if the filter works
+fixed in/out direction filter testcase: sum of in and
out calls should be >= total count of calls

Change-Id: Ibe4b8153f023c53bb463626ad9e6613992fc101d

lib/NGCP/Panel/Controller/API/CallLists.pm

@@ -33,16 +33,13 @@ class_has 'query_params' => (
             query => {
                 first => sub {
                     my $q = shift;
-                    return {
-                        -or => [
-                            'source_subscriber.id' => $q,
-                            'destination_subscriber.id' => $q, 
-                        ],
+                    return { 
+                        'subscriber.id' => $q,
                     };
                 },
                 second => sub {
                     return {
-                        join => ['source_subscriber', 'destination_subscriber'],
+                        join => 'subscriber',
                     };
                 },
             },
@@ -145,7 +142,6 @@ class_has 'query_params' => (
                         } else {
                             return {
                                 'destination_account_id' => $owner->{customer}->id,
-                                'source_account_id' => {'!=' => $owner->{customer}->id},
                             };
                         }
                     }

lib/NGCP/Panel/Controller/API/Calls.pm

@@ -49,16 +49,13 @@ class_has 'query_params' => (
             query => {
                 first => sub {
                     my $q = shift;
-                    return {
-                        -or => [
-                            { 'source_subscriber.id' => $q },
-                            { 'destination_subscriber.id' => $q }
-                        ],
+                    return { 
+                        'subscriber.id' => $q,
                     };
                 },
                 second => sub {
                     return {
-                        join => ['source_subscriber', 'destination_subscriber'],
+                        join => 'subscriber',
                     };
                 },
             },

lib/NGCP/Panel/Role/API/CallLists.pm

@@ -38,13 +38,10 @@ sub item_rs {
             ],
         });
     } else {
-        $item_rs = $item_rs->search({ 
-            -or => [
-                { 'source_subscriber.id' => $c->user->voip_subscriber->id },
-                { 'destination_subscriber.id' => $c->user->voip_subscriber->id },
-            ],
+        $item_rs = $item_rs->search({
+            'subscriber.id' => $c->user->voip_subscriber->id,
         },{
-            join => ['source_subscriber', 'destination_subscriber'],
+            join => 'subscriber',
         });
     }
     $item_rs = $item_rs->search({

t/api-rest/api-calllists.t

@@ -32,8 +32,8 @@ SKIP:
 
     ($res, $cl_collection_out) = $test_machine->check_item_get('/api/calllists/?page=1&rows=10&direction=out&subscriber_id='.$sub1_id, "fetch calllists collection of subscriber ($sub1_id) with direction filter out");
 
-    is($cl_collection_in->{total_count}+$cl_collection_out->{total_count}, $cl_collection->{total_count},
-        "Incoming and outgoing calls should add up to total number of calls");
+    ok($cl_collection_in->{total_count} + $cl_collection_out->{total_count} >= $cl_collection->{total_count},
+        "Incoming and outgoing calls should be greater than or equal to total number of calls");
 
     diag("Total number of calls: " . $cl_collection->{total_count});
 }
@@ -60,12 +60,44 @@ SKIP:
 
     ($res, $cl_collection_out) = $test_machine->check_item_get('/api/calllists/?page=1&rows=10&direction=out&customer_id='.$cust1_id,"fetch calllists collection of customer ($cust1_id) with direction filter out");
 
-    is($cl_collection_in->{total_count}+$cl_collection_out->{total_count}, $cl_collection->{total_count},
-        "Incoming and outgoing calls should add up to total number of calls");
+    ok($cl_collection_in->{total_count} + $cl_collection_out->{total_count} >= $cl_collection->{total_count},
+        "Incoming and outgoing calls should be greater than or equal to total number of calls");
 
     diag("Total number of calls: " . $cl_collection->{total_count});
 }
 
+SKIP:
+{ #MT#16171
+    my ($res,$sub1,$sub1_id,$sub1_user,$sub1_pass,$cl_collection, $cl_collection_in, $cl_collection_out);
+
+    ($res, $sub1) = $test_machine->check_item_get('/api/subscribers/?page=1&rows=1&order_by=id&order_by_direction=desc',"fetch a subscriber for testing");
+    if ($sub1->{total_count} < 1) {
+        skip("Precondition not met: need a subscriber",1);
+    }
+    ($sub1_id) = $sub1->{_embedded}->{'ngcp:subscribers'}->{_links}{self}{href} =~ m!subscribers/([0-9]*)$!;
+    #$sub1_user = $sub1->{_embedded}->{'ngcp:subscribers'}->{'username'};
+    #$sub1_pass = $sub1->{_embedded}->{'ngcp:subscribers'}->{'webpassword'} // '';
+
+    cmp_ok ($sub1_id, '>', 0, "should be positive integer");
+    
+    ($res, $cl_collection) = $test_machine->check_item_get('/api/calls/?page=1&rows=10&subscriber_id='.$sub1_id,"fetch calls collection of subscriber ($sub1_id) by filter");
+    
+    diag("Total number of calls: " . $cl_collection->{total_count});
+    
+    #subscriber api login only works if panel session is established.
+    
+    #$test_machine = Test::Collection->new(
+    #    name => 'subscriber_calls',
+    #    runas_role => 'subscriber',
+    #    subscriber_user => $sub1_user,
+    #    subscriber_pass => $sub1_pass,
+    #);
+    
+    #($res, $cl_collection) = $test_machine->check_item_get('/api/calls/?page=1&rows=10',"fetch calls collection of subscriber ($sub1_id) as subscriber");
+    
+    #diag("Total number of calls: " . $cl_collection->{total_count});
+    
+}
 
 done_testing;
 

t/api-rest/api-calls.t

@@ -0,0 +1,33 @@
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Collection;
+
+my $test_machine = Test::Collection->new(
+    name => 'calls',
+);
+
+diag('Note that the next tests require at least one subscriber to be present');
+
+SKIP:
+{ #MT#16171
+    my ($res,$sub1,$sub1_id,$sub1_user,$sub1_pass,$cl_collection, $cl_collection_in, $cl_collection_out);
+
+    ($res, $sub1) = $test_machine->check_item_get('/api/subscribers/?page=1&rows=1&order_by=id&order_by_direction=desc',"fetch a subscriber for testing");
+    if ($sub1->{total_count} < 1) {
+        skip("Precondition not met: need a subscriber",1);
+    }
+    ($sub1_id) = $sub1->{_embedded}->{'ngcp:subscribers'}->{_links}{self}{href} =~ m!subscribers/([0-9]*)$!;
+
+    cmp_ok ($sub1_id, '>', 0, "should be positive integer");
+    
+    ($res, $cl_collection) = $test_machine->check_item_get('/api/calls/?page=1&rows=10&subscriber_id='.$sub1_id,"fetch calls collection of subscriber ($sub1_id) by filter");
+    
+    diag("Total number of calls: " . $cl_collection->{total_count});
+    
+}
+
+done_testing;
+
+# vim: set tabstop=4 expandtab:

t/lib/Test/Collection.pm

@@ -63,6 +63,14 @@ has 'name' => (
     is => 'rw',
     isa => 'Str',
 );
+#has 'subscriber_user' => (
+#    is => 'rw',
+#    isa => 'Str',
+#);
+#has 'subscriber_pass' => (
+#    is => 'rw',
+#    isa => 'Str',
+#);
 has 'embedded_resources' => (
     is => 'rw',
     isa => 'ArrayRef',
@@ -224,14 +232,20 @@ sub get_role_credentials{
     my($role) = @_;
     my($user,$pass);
     $role //= $self->runas_role // 'default';
+    my $realm;
     if($role eq 'default' || $role eq 'admin'){
         $user //= $ENV{API_USER} // 'administrator';
         $pass //= $ENV{API_PASS} // 'administrator';
+        $realm = 'api_admin_http';
     }elsif($role eq 'reseller'){
         $user //= $ENV{API_USER_RESELLER} // 'api_test';
         $pass //= $ENV{API_PASS_RESELLER} // 'api_test';
+        $realm = 'api_admin_http';
+    #}elsif($role eq 'subscriber'){
+    #    $user //= $self->subscriber_user;
+    #    $pass //= $self->subscriber_pass;
+    #    $realm = 'subscriber';
     }
-    my $realm = 'api_admin_http';
     return($user,$pass,$role,$realm);
 }
 sub clear_data_created{