From cd2d9e47aa0377ecec5165b4885629c9560df763 Mon Sep 17 00:00:00 2001 From: Andreas Granig Date: Fri, 6 Dec 2013 22:50:58 +0100 Subject: [PATCH] MT#5255 Fix boolean handling and HFH validation --- .../Panel/Controller/API/BillingProfiles.pm | 1 + .../Controller/API/BillingProfilesItem.pm | 8 ++++-- lib/NGCP/Panel/Controller/API/Contracts.pm | 1 + .../Panel/Controller/API/ContractsItem.pm | 6 +++-- lib/NGCP/Panel/Role/API.pm | 26 ++++++++++++------- lib/NGCP/Panel/Role/API/BillingProfiles.pm | 2 ++ lib/NGCP/Panel/Role/API/Contracts.pm | 2 ++ t/api-billingprofiles.t | 10 +++---- 8 files changed, 37 insertions(+), 19 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/BillingProfiles.pm b/lib/NGCP/Panel/Controller/API/BillingProfiles.pm index 3204baf89e..d1d5b21eae 100644 --- a/lib/NGCP/Panel/Controller/API/BillingProfiles.pm +++ b/lib/NGCP/Panel/Controller/API/BillingProfiles.pm @@ -145,6 +145,7 @@ sub POST :Allow { } my $form = NGCP::Panel::Form::BillingProfile::Admin->new; + $resource->{reseller_id} //= undef; last unless $self->validate_form( c => $c, resource => $resource, diff --git a/lib/NGCP/Panel/Controller/API/BillingProfilesItem.pm b/lib/NGCP/Panel/Controller/API/BillingProfilesItem.pm index ce057c30b5..79b8f5c193 100644 --- a/lib/NGCP/Panel/Controller/API/BillingProfilesItem.pm +++ b/lib/NGCP/Panel/Controller/API/BillingProfilesItem.pm @@ -106,7 +106,8 @@ sub PATCH :Allow { last unless $resource; my $form = NGCP::Panel::Form::BillingProfile::Admin->new; - last unless $self->update_profile($c, $profile, $old_resource, $resource, $form); + $profile = $self->update_profile($c, $profile, $old_resource, $resource, $form); + last unless $profile; $guard->commit; @@ -145,7 +146,10 @@ sub PUT :Allow { my $old_resource = { $profile->get_inflated_columns }; my $form = NGCP::Panel::Form::BillingProfile::Admin->new; - last unless $self->update_profile($c, $profile, $old_resource, $resource, $form); + use Data::Printer; p $profile; + $profile = $self->update_profile($c, $profile, $old_resource, $resource, $form); + use Data::Printer; p $profile; + last unless $profile; $guard->commit; diff --git a/lib/NGCP/Panel/Controller/API/Contracts.pm b/lib/NGCP/Panel/Controller/API/Contracts.pm index a06ed22a96..fc2b11f121 100644 --- a/lib/NGCP/Panel/Controller/API/Contracts.pm +++ b/lib/NGCP/Panel/Controller/API/Contracts.pm @@ -145,6 +145,7 @@ sub POST :Allow { last unless $resource; my $product_class = delete $resource->{type}; + $resource->{contact_id} //= undef; my $form = NGCP::Panel::Form::Contract::PeeringReseller->new; last unless $self->validate_form( c => $c, diff --git a/lib/NGCP/Panel/Controller/API/ContractsItem.pm b/lib/NGCP/Panel/Controller/API/ContractsItem.pm index f06151ffbd..105a75f4d8 100644 --- a/lib/NGCP/Panel/Controller/API/ContractsItem.pm +++ b/lib/NGCP/Panel/Controller/API/ContractsItem.pm @@ -108,7 +108,8 @@ sub PATCH :Allow { last unless $resource; my $form = NGCP::Panel::Form::Contract::PeeringReseller->new; - last unless $self->update_contract($c, $contract, $old_resource, $resource, $form); + $contract = $self->update_contract($c, $contract, $old_resource, $resource, $form); + last unless $contract; $guard->commit; @@ -147,7 +148,8 @@ sub PUT :Allow { my $old_resource = { $contract->get_inflated_columns }; my $form = NGCP::Panel::Form::Contract::PeeringReseller->new; - last unless $self->update_contract($c, $contract, $old_resource, $resource, $form); + $contract = $self->update_contract($c, $contract, $old_resource, $resource, $form); + last unless $contract; $guard->commit; diff --git a/lib/NGCP/Panel/Role/API.pm b/lib/NGCP/Panel/Role/API.pm index 7db8617d78..3e8aa7c25a 100644 --- a/lib/NGCP/Panel/Role/API.pm +++ b/lib/NGCP/Panel/Role/API.pm @@ -101,20 +101,26 @@ sub validate_form { $form->field($k)->$_isa('HTML::FormHandler::Field::Integer') || $form->field($k)->$_isa('HTML::FormHandler::Field::Money') || $form->field($k)->$_isa('HTML::FormHandler::Field::Float'))); - $resource->{$k} = JSON::Types::bool($resource->{$k}) - if(defined $resource->{$k} && - $form->field($k)->$_isa('HTML::FormHandler::Field::Boolean')); + + # only do this for converting back from obj to hal + # otherwise it breaks db fields with the \0 and \1 notation + unless($run) { + $resource->{$k} = JSON::Types::bool($resource->{$k}) + if(defined $resource->{$k} && + $form->field($k)->$_isa('HTML::FormHandler::Field::Boolean')); + } } if($run) { # check keys/vals - my $result = $form->run(params => $resource); - use Data::Printer; p $result; p $result->error_results; - if ($result->error_results->size) { - my $f = $result->error_results->[0]; - my $e = $result->error_results->map(sub { - sprintf 'field=\'%s\', input=\'%s\', errors=\'%s\'', ($_->parent->$_isa('HTML::FormHandler::Field::Result') ? $_->parent->name . '_' : '') . $_->name, $_->input // '', $_->errors->join(q()) - })->join("; "); + $form->process(params => $resource, posted => 1); + unless($form->validated) { + my $e = join '; ', map { + sprintf 'field=\'%s\', input=\'%s\', errors=\'%s\'', + ($_->parent->$_isa('HTML::FormHandler::Field') ? $_->parent->name . '_' : '') . $_->name, + $_->input // '', + $_->errors->join(q()) + } $form->error_fields; $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Validation failed. $e"); return; } diff --git a/lib/NGCP/Panel/Role/API/BillingProfiles.pm b/lib/NGCP/Panel/Role/API/BillingProfiles.pm index 72d72cfd68..f57cb28508 100644 --- a/lib/NGCP/Panel/Role/API/BillingProfiles.pm +++ b/lib/NGCP/Panel/Role/API/BillingProfiles.pm @@ -71,6 +71,8 @@ sub update_profile { my ($self, $c, $profile, $old_resource, $resource, $form) = @_; $form //= NGCP::Panel::Form::BillingProfile::Admin->new; + # TODO: for some reason, formhandler lets missing reseller slip thru + $resource->{reseller_id} //= undef; return unless $self->validate_form( c => $c, form => $form, diff --git a/lib/NGCP/Panel/Role/API/Contracts.pm b/lib/NGCP/Panel/Role/API/Contracts.pm index 51ee11eec3..376bc4c287 100644 --- a/lib/NGCP/Panel/Role/API/Contracts.pm +++ b/lib/NGCP/Panel/Role/API/Contracts.pm @@ -102,6 +102,8 @@ sub update_contract { $resource->{billing_profile_id} //= $old_resource->{billing_profile_id}; $form //= NGCP::Panel::Form::Contract::PeeringReseller->new; + # TODO: for some reason, formhandler lets missing contact_id slip thru + $resource->{contact_id} //= undef; return unless $self->validate_form( c => $c, form => $form, diff --git a/t/api-billingprofiles.t b/t/api-billingprofiles.t index dbd8f44548..b2b2d6fbae 100644 --- a/t/api-billingprofiles.t +++ b/t/api-billingprofiles.t @@ -49,8 +49,8 @@ my @allprofiles = (); $req->header('Content-Type' => 'application/json'); $req->content(JSON::to_json({ reseller_id => $reseller_id, - handle => "testapihandle$i", - name => "test api name $i", + handle => "testapihandle$i".time, + name => "test api name $i".time, })); $res = $ua->request($req); ok($res->code == 201, "create test billing profile $i"); @@ -274,14 +274,14 @@ my @allprofiles = (); $req = HTTP::Request->new('PATCH', $uri.'/'.$firstprofile); $req->header('Prefer' => 'return=representation'); $req->header('Content-Type' => 'application/json-patch+json'); - + my $t = time; $req->content(JSON::to_json( - [ { op => 'replace', path => '/name', value => 'patched name' } ] + [ { op => 'replace', path => '/name', value => 'patched name '.$t } ] )); $res = $ua->request($req); ok($res->code == 200, "check patched profile item"); my $mod_profile = JSON::from_json($res->decoded_content); - ok($mod_profile->{name} eq "patched name", "check patched replace op"); + ok($mod_profile->{name} eq "patched name $t", "check patched replace op"); ok($mod_profile->{_links}->{self}->{href} eq $firstprofile, "check patched self link"); ok($mod_profile->{_links}->{collection}->{href} eq '/api/billingprofiles/', "check patched collection link");