From 1d9907ad45572d774bca7813e694dcf899b3a171 Mon Sep 17 00:00:00 2001
From: Irina Peshinskaya <ipeshinskaya@sipwise.com>
Date: Fri, 20 Apr 2018 00:39:26 +0200
Subject: [PATCH] TT#35429 Allow both 0/1 an true/false as boolean input in
 /api/admins

Change-Id: I4c9805ebce17586674f52a9fe97f280fb9991612
---
 lib/NGCP/Panel/Controller/API/Admins.pm |  4 +-
 lib/NGCP/Panel/Role/API/Admins.pm       |  6 ++-
 t/api-rest/api-admins-collection.t      | 52 +++++++++++++++++++++++++
 t/api-rest/api-phonebookentries.t       |  7 +---
 t/lib/Test/FakeData.pm                  | 25 ++++++------
 5 files changed, 74 insertions(+), 20 deletions(-)
 create mode 100644 t/api-rest/api-admins-collection.t

diff --git a/lib/NGCP/Panel/Controller/API/Admins.pm b/lib/NGCP/Panel/Controller/API/Admins.pm
index 1f5324456f..c4399ead98 100644
--- a/lib/NGCP/Panel/Controller/API/Admins.pm
+++ b/lib/NGCP/Panel/Controller/API/Admins.pm
@@ -36,7 +36,7 @@ sub create_item {
     my ($self, $c, $resource, $form, $process_extras) = @_;
     unless($c->user->is_master) {
         $self->error($c, HTTP_FORBIDDEN, "Cannot create admin without master permissions");
-        last;
+        return;
     }
     my $item;
     try {
@@ -44,7 +44,7 @@ sub create_item {
     } catch($e) {
         $c->log->error("failed to create admin: $e");
         $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to create admin.");
-        last;
+        return;
     }
     return $item;
 }
diff --git a/lib/NGCP/Panel/Role/API/Admins.pm b/lib/NGCP/Panel/Role/API/Admins.pm
index 815c65fb6e..c8f736d055 100644
--- a/lib/NGCP/Panel/Role/API/Admins.pm
+++ b/lib/NGCP/Panel/Role/API/Admins.pm
@@ -79,6 +79,10 @@ sub process_form_resource{
         $resource->{md5pass} = undef;
         $resource->{saltedpass} = NGCP::Panel::Utils::Admin::generate_salted_hash($pass);
     }
+    foreach my $f(qw/billing_data call_data is_active is_master is_superuser lawful_intercept read_only show_passwords/) {
+        $resource->{$f} = (ref $resource->{$f} eq 'JSON::true' ) ? 1 : 0;
+        #|| $resource->{$f} eq 'true'
+    }
     return $resource;
 }
 
@@ -98,7 +102,7 @@ sub check_duplicate{
     if ($existing_item && (!$item || $item->id != $existing_item->id)) {
         $c->log->error("admin with login '$$resource{login}' already exists");
         $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Admin with this login already exists");
-        last;
+        return;
     }
     return 1;
 }
diff --git a/t/api-rest/api-admins-collection.t b/t/api-rest/api-admins-collection.t
new file mode 100644
index 0000000000..1ec95adfcc
--- /dev/null
+++ b/t/api-rest/api-admins-collection.t
@@ -0,0 +1,52 @@
+use strict;
+use warnings;
+
+use Test::Collection;
+use Test::FakeData;
+use Test::More;
+use Data::Dumper;
+
+#init test_machine
+my $test_machine = Test::Collection->new(
+    name => 'admins',
+);
+$test_machine->methods->{collection}->{allowed} = {map {$_ => 1} qw(GET HEAD OPTIONS POST)};
+$test_machine->methods->{item}->{allowed}       = {map {$_ => 1} qw(GET HEAD OPTIONS DELETE)};
+
+my $fake_data =  Test::FakeData->new;
+$fake_data->set_data_from_script({
+    'admins' => {
+        'data' => {
+            "reseller_id"  =>  sub { return shift->get_id('resellers',@_); },
+            "login"     => 'api_test_admin',
+            "password"  => 'api_test_admin',
+            "is_active" => 'true',
+            "is_master" => 'true',
+            "is_superuser" => 'true',
+            "billing_data" => 'true',
+            "call_data" => 'false',
+            "lawful_intercept" => 'false',
+            "read_only" => 'false',
+            "show_passwords" => 'true'
+        },
+        'query' => ['login'],
+        'data_callbacks' => {
+            'uniquizer_cb' => sub { Test::FakeData::string_uniquizer(\$_[0]->{login}); },
+        },
+    },
+});
+
+#for item creation test purposes /post request data/
+$test_machine->DATA_ITEM_STORE($fake_data->process('admins'));
+
+$test_machine->form_data_item( );
+# create 3 new admins from DATA_ITEM
+my $admins = $test_machine->check_create_correct( 3 , sub {
+    $_[0]->{login}.=seq()
+});
+$test_machine->check_bundle();
+$test_machine->clear_test_data_all();
+
+done_testing;
+
+# vim: set tabstop=4 expandtab:
diff --git a/t/api-rest/api-phonebookentries.t b/t/api-rest/api-phonebookentries.t
index 98769496a9..397435f271 100644
--- a/t/api-rest/api-phonebookentries.t
+++ b/t/api-rest/api-phonebookentries.t
@@ -320,10 +320,5 @@ sub test_phonebook_collection{
     $test_machine->content_type(clone $content_type_old);
 }
 
-sub seq{
-    my ($number) = @_;
-    state $seq = 0;
-    $number //= 0;
-    return $number + $seq++;
-}
+
 # vim: set tabstop=4 expandtab:
diff --git a/t/lib/Test/FakeData.pm b/t/lib/Test/FakeData.pm
index 8d6d4300ab..a37d0c766c 100644
--- a/t/lib/Test/FakeData.pm
+++ b/t/lib/Test/FakeData.pm
@@ -1,7 +1,9 @@
 package Test::FakeData;
 
 use strict;
+use warnings;
 
+use Moose::Exporter;
 use Moose;
 use Test::Collection;
 use JSON;
@@ -18,6 +20,11 @@ use feature 'state';
 use Storable;
 use File::Temp qw(tempfile);
 
+
+Moose::Exporter->setup_import_methods(
+    as_is     => [ 'seq' ],
+);
+
 sub BUILD {
     my $self = shift;
     if($self->test_machine->cache_data){
@@ -189,17 +196,6 @@ sub build_data_default{
 sub build_data{
     my ($self) = @_;
     my $data = {
-        'admins' => {
-            'data' => {
-                login       => 'api_test_admin',
-                password    => 'api_test_admin',
-                reseller_id => sub { return shift->get_id('resellers',@_); },
-            },
-            'query' => ['login'],
-            'data_callbacks' => {
-                'uniquizer_cb' => sub { Test::FakeData::string_uniquizer(\$_[0]->{login}); },
-            },
-        },
         'applyrewrites' => {
             'data' => {
                 direction => "caller_in",
@@ -889,6 +885,13 @@ sub DEMOLISH{
     }
 }
 
+sub seq{
+    my ($number) = @_;
+    state $seq = 0;
+    $number //= 0;
+    return $number + $seq++;
+}
+
 1;
 
 __END__