@@ -, +, @@ - Attribute mapping logic is now on the Koha::Object-level (the patches implement that, but are not using it) - Related to the above, some helper methods like to_api and to_model are kept, the same for the mappings in the controller, they should all go away - Related to the above, set_from_api and new_from_api should be used instead of using helper to_api and to_model methods in the controller - $c->objects->search doesn't use the to_model and to_api params - Response status codes need to be changed, at least for DELETE operations $ kshell k$ prove t/db_dependent/api/v1/advanced_editor_macros.t --- Koha/REST/V1/AdvancedEditorMacro.pm | 180 +++++------------- .../swagger/paths/advancededitormacros.json | 4 +- .../api/v1/advanced_editor_macros.t | 12 +- 3 files changed, 53 insertions(+), 143 deletions(-) --- a/Koha/REST/V1/AdvancedEditorMacro.pm +++ a/Koha/REST/V1/AdvancedEditorMacro.pm @@ -21,11 +21,13 @@ use Koha::AdvancedEditorMacros; use Try::Tiny; -=head1 API +=head1 Name -=head2 Class Methods +Koha::REST::V1::AdvancedEditorMacro -=cut +=head1 API + +=head2 Methods =head3 list @@ -37,19 +39,20 @@ sub list { my $c = shift->openapi->valid_input or return; my $patron = $c->stash('koha.user'); return try { - my $macros_set = Koha::AdvancedEditorMacros->search({ -or => { shared => 1, borrowernumber => $patron->borrowernumber } }); - my $macros = $c->objects->search( $macros_set, \&_to_model, \&_to_api ); - return $c->render( status => 200, openapi => $macros ); + my $macros_set = Koha::AdvancedEditorMacros->search( + { + -or => + { shared => 1, borrowernumber => $patron->borrowernumber } + } + ); + my $macros = $c->objects->search( $macros_set ); + return $c->render( + status => 200, + openapi => $macros + ); } catch { - if ( $_->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $_->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs. $_"} ); - } + $c->unhandled_exception($_); }; } @@ -123,15 +126,17 @@ sub add { } return try { - my $macro = Koha::AdvancedEditorMacro->new( _to_model( $c->validation->param('body') ) ); - $macro->store; + my $macro = Koha::AdvancedEditorMacro->new_from_api( $c->validation->param('body') ); + $macro->store->discard_changes; $c->res->headers->location( $c->req->url->to_string . '/' . $macro->id ); return $c->render( status => 201, openapi => $macro->to_api ); } - catch { handle_error($_) }; + catch { + $c->unhandled_exception($_); + }; } =head3 add_shared @@ -148,15 +153,17 @@ sub add_shared { openapi => { error => "To create private macros you must use advancededitor" } ); } return try { - my $macro = Koha::AdvancedEditorMacro->new( _to_model( $c->validation->param('body') ) ); - $macro->store; + my $macro = Koha::AdvancedEditorMacro->new_from_api( $c->validation->param('body') ); + $macro->store->discard_changes; $c->res->headers->location( $c->req->url->to_string . '/' . $macro->id ); return $c->render( status => 201, openapi => $macro->to_api ); } - catch { handle_error($_) }; + catch { + $c->unhandled_exception($_); + }; } =head3 update @@ -188,11 +195,13 @@ sub update { return try { my $params = $c->req->json; - $macro->set( _to_model($params) ); - $macro->store(); + $macro->set_from_api( $params ); + $macro->store->discard_changes; return $c->render( status => 200, openapi => $macro->to_api ); } - catch { handle_error($_) }; + catch { + $c->unhandled_exception($_); + }; } =head3 update_shared @@ -218,11 +227,13 @@ sub update_shared { return try { my $params = $c->req->json; - $macro->set( _to_model($params) ); - $macro->store(); + $macro->set_from_api( $params ); + $macro->store->discard_changes; return $c->render( status => 200, openapi => $macro->to_api ); } - catch { handle_error($_) }; + catch { + $c->unhandled_exception($_); + }; } =head3 delete @@ -253,9 +264,11 @@ sub delete { return try { $macro->delete; - return $c->render( status => 200, openapi => "" ); + return $c->render( status => 204, openapi => q{} ); } - catch { handle_error($_) }; + catch { + $c->unhandled_exception($_); + }; } =head3 delete_shared @@ -280,114 +293,11 @@ sub delete_shared { return try { $macro->delete; - return $c->render( status => 200, openapi => "" ); - } - catch { handle_error($_,$c) }; -} - -=head3 _handle_error - -Helper function that passes exception or error - -=cut - -sub _handle_error { - my ($err,$c) = @_; - if ( $err->isa('DBIx::Class::Exception') ) { - return $c->render( status => 500, - openapi => { error => $err->{msg} } ); - } - else { - return $c->render( status => 500, - openapi => { error => "Something went wrong, check the logs."} ); - } -}; - - -=head3 _to_api - -Helper function that maps a hashref of Koha::AdvancedEditorMacro attributes into REST api -attribute names. - -=cut - -sub _to_api { - my $macro = shift; - - # Rename attributes - foreach my $column ( keys %{ $Koha::REST::V1::AdvancedEditorMacro::to_api_mapping } ) { - my $mapped_column = $Koha::REST::V1::AdvancedEditorMacro::to_api_mapping->{$column}; - if ( exists $macro->{ $column } - && defined $mapped_column ) - { - # key /= undef - $macro->{ $mapped_column } = delete $macro->{ $column }; - } - elsif ( exists $macro->{ $column } - && !defined $mapped_column ) - { - # key == undef => to be deleted - delete $macro->{ $column }; - } - } - - return $macro; -} - -=head3 _to_model - -Helper function that maps REST api objects into Koha::AdvancedEditorMacros -attribute names. - -=cut - -sub _to_model { - my $macro = shift; - - foreach my $attribute ( keys %{ $Koha::REST::V1::AdvancedEditorMacro::to_model_mapping } ) { - my $mapped_attribute = $Koha::REST::V1::AdvancedEditorMacro::to_model_mapping->{$attribute}; - if ( exists $macro->{ $attribute } - && defined $mapped_attribute ) - { - # key /= undef - $macro->{ $mapped_attribute } = delete $macro->{ $attribute }; - } - elsif ( exists $macro->{ $attribute } - && !defined $mapped_attribute ) - { - # key == undef => to be deleted - delete $macro->{ $attribute }; - } + return $c->render( status => 204, openapi => q{} ); } - - if ( exists $macro->{shared} ) { - $macro->{shared} = ($macro->{shared}) ? 1 : 0; - } - - - return $macro; + catch { + $c->unhandled_exception($_); + }; } -=head2 Global variables - -=head3 $to_api_mapping - -=cut - -our $to_api_mapping = { - id => 'macro_id', - macro => 'macro_text', - borrowernumber => 'patron_id', -}; - -=head3 $to_model_mapping - -=cut - -our $to_model_mapping = { - macro_id => 'id', - macro_text => 'macro', - patron_id => 'borrowernumber', -}; - 1; --- a/api/v1/swagger/paths/advancededitormacros.json +++ a/api/v1/swagger/paths/advancededitormacros.json @@ -308,7 +308,7 @@ "application/json" ], "responses": { - "200": { + "204": { "description": "Advanced editor macro deleted", "schema": { "type": "string" @@ -477,7 +477,7 @@ "application/json" ], "responses": { - "200": { + "204": { "description": "Advanced editor macro deleted", "schema": { "type": "string" --- a/t/db_dependent/api/v1/advanced_editor_macros.t +++ a/t/db_dependent/api/v1/advanced_editor_macros.t @@ -151,7 +151,7 @@ subtest 'get() tests' => sub { $t->get_ok( "//$userid:$password@/api/v1/advanced_editor/macros/shared/" . $macro_1->id ) ->status_is( 200, 'Can get a shared macro via shared endpoint' ) - ->json_is( '' => Koha::REST::V1::AdvancedEditorMacro::_to_api( $macro_1->TO_JSON ), 'Macro correctly retrieved' ); + ->json_is( $macro_1->to_api ); $t->get_ok( "//$userid:$password@/api/v1/advanced_editor/macros/" . $macro_2->id ) ->status_is( 403, 'Cannot access another users macro' ) @@ -159,7 +159,7 @@ subtest 'get() tests' => sub { $t->get_ok( "//$userid:$password@/api/v1/advanced_editor/macros/" . $macro_3->id ) ->status_is( 200, 'Can get your own private macro' ) - ->json_is( '' => Koha::REST::V1::AdvancedEditorMacro::_to_api( $macro_3->TO_JSON ), 'Macro correctly retrieved' ); + ->json_is( $macro_3->to_api ); my $non_existent_code = $macro_1->id; $macro_1->delete; @@ -202,7 +202,7 @@ subtest 'add() tests' => sub { class => 'Koha::AdvancedEditorMacros', value => { shared => 0 } }); - my $macro_values = Koha::REST::V1::AdvancedEditorMacro::_to_api( $macro->TO_JSON ); + my $macro_values = $macro->to_api; delete $macro_values->{macro_id}; $macro->delete; @@ -317,7 +317,7 @@ subtest 'update() tests' => sub { }); my $macro_id = $macro->id; my $macro_2_id = $macro_2->id; - my $macro_values = Koha::REST::V1::AdvancedEditorMacro::_to_api( $macro->TO_JSON ); + my $macro_values = $macro->to_api; delete $macro_values->{macro_id}; # Unauthorized attempt to update @@ -448,7 +448,7 @@ subtest 'delete() tests' => sub { ->status_is(403, "Cannot delete macro without permission"); $t->delete_ok( "//$auth_userid:$password@/api/v1/advanced_editor/macros/$macro_id") - ->status_is(200, 'Can delete macro with permission'); + ->status_is( 204, 'Can delete macro with permission'); $t->delete_ok( "//$auth_userid:$password@/api/v1/advanced_editor/macros/$macro_2_id") ->status_is(403, 'Cannot delete other users macro with permission'); @@ -469,7 +469,7 @@ subtest 'delete() tests' => sub { $t->delete_ok( "//$auth_userid:$password@/api/v1/advanced_editor/macros/$macro_2_id") ->status_is(403, 'Cannot delete other users shared macro with permission on private endpoint'); $t->delete_ok( "//$auth_userid:$password@/api/v1/advanced_editor/macros/shared/$macro_2_id") - ->status_is(200, 'Can delete other users shared macro with permission'); + ->status_is(204, 'Can delete other users shared macro with permission'); }; --