Bug 38050 - /lists

[mrenvoize]

No description provided.

[mrenvoize]

We'll also need unit tests.. examples in t/db_dependent/api

[mrenvoize]

This whole routine is rather custom without any real need for it to be.. please follow the patterns set in all the other API controllers... i.e. using the API helpers so we get all the search and query handling from there.

[mrenvoize]

You'll want to follow Tomas's example to some extent here and use:

my $lists_set = Koha::Virtualshelves->new;
$lists_set = $lists_set->filter_by_readable( { patron_id => $user->id } );

return $c->render(
    status  => 200,
    openapi => $c->objects->search($lists_set),
);

He's also got error handling in a wrapper.. you'll likely want that too for various failure cases the object searches can throw.

[mrenvoize]

Please call this method 'get'.. 'read' is a homonym for a Perl builtin so can sometimes be confused... also, that would make this consistent with other controllers.

[mrenvoize]

Please call this 'add' rather than 'create' for consistency with other API controllers.

[mrenvoize]

This is another case where you've not used the foundations that have been laid out before you.

See the 'Cities' controller for reference as it's generally the most basic 'best practices' example... I should have pointed you to that.. assuming I didn't already.

sub add {
    my $c = shift->openapi->valid_input or return;

    return try {
        my $city = Koha::City->new_from_api( $c->req->json );
        $city->store;
        $c->res->headers->location( $c->req->url->to_string . '/' . $city->cityid );
        return $c->render(
            status  => 201,
            openapi => $c->objects->to_api($city),
        );
    }
    catch {
        $c->unhandled_exception($_);
    };
}

[mrenvoize]

As above.. by using 'json' as your output directly, you're skipping all validation... please use the 'openapi' output handler as that include validation against the specification files.

[mrenvoize]

There's a pattern for this one too:

sub get {
    my $c = shift->openapi->valid_input or return;

    return try {
        my $city = Koha::Cities->find( $c->param('city_id') );
        return $c->render_resource_not_found("City")
            unless $city;

        return $c->render( status => 200, openapi => $c->objects->to_api($city), );
    } catch {
        $c->unhandled_exception($_);
    };
}

I think you'll likely need to use the above mentioned filter_by_readable method however to ensure only those with appropriate access permissions can 'get' the specific list.

[mrenvoize]

There's another template for this:

sub update {
    my $c = shift->openapi->valid_input or return;

    my $city = Koha::Cities->find( $c->param('city_id') );

    return $c->render_resource_not_found("City")
        unless $city;

    return try {
        $city->set_from_api( $c->req->json );
        $city->store();
        return $c->render( status => 200, openapi => $c->objects->to_api($city), );
    }
    catch {
        $c->unhandled_exception($_);
    };
}

We'll want to think about how we handle adding and removing biblios to/from lists.. lets keep the basic crud at the list level only however.

[mrenvoize]

Again.. there's a template:

sub delete {
    my $c = shift->openapi->valid_input or return;

    my $city = Koha::Cities->find( $c->param('city_id') );

    return $c->render_resource_not_found("City")
        unless $city;

    return try {
        $city->delete;
        return $c->render_resource_deleted;
    }
    catch {
        $c->unhandled_exception($_);
    };
}