[Koha-bugs] [Bug 18137] REST API: Migrate from Mojolicious::Plugin:: Swagger2 to Mojolicious::Plugin::OpenAPI

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Thu Aug 24 00:14:32 CEST 2017


https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18137

--- Comment #100 from Tomás Cohen Arazi <tomascohen at gmail.com> ---
(In reply to Martin Renvoize from comment #99)
> Comment on attachment 65107 [details] [review]
> Bug 18137: Migrate from Swagger2 to Mojolicious::Plugin::OpenAPI
> 
> Review of attachment 65107 [details] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/REST/V1/Auth.pm
> @@ +57,5 @@
> > +                json => { error => 'Something went wrong, check the logs.' }
> > +            );
> > +        }
> > +        if ($_->isa('Koha::Exceptions::UnderMaintenance')) {
> > +            return $c->render(status => 503, json => { error => $_->error });
> 
> Should these calls to render not all be 'openapi => { error => $_->error }`
> as aposed to `json => { error => $_->error }`.. else none of these responses
> are getting validated.

That's because the auth check is done using ->under, and as the check itself is
calling ->render, it is breaking the dispatch chain (not breaking like in 'stop
working' but like short-circuiting to another way of returning the error.
Clarifying to avoid possible idiomatic issues).

I think we should leave it as-is and deal with this problem on a separate bug,
or leave it. Because it could involve a big re-engineering to make the check
let the chain continue while propagating the exception.

I can't even imagine how to do it without adding irrelevant checks and
repetitive code to controller classes.

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list