Commit Graph

386 Commits

Author SHA1 Message Date
Tomas Cohen Arazi 78b5f207cb Bug 29806: Fix POST /holds use of pickup_locations
This patch makes Koha::Item->pickup_locations and
Koha::Biblio->pickup_locations explicitly call ->as_list in list
context. This way we workaround bug 28871.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail, the route gives a 500 (unhandled exception)
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass, the route correctly returns a 400 with a
message about the pickup location being invalid
5. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-02-07 06:40:36 -05:00
Tomas Cohen Arazi 7341637d93 Bug 29464: Make GET /acquisitions/orders honour sorting
When bug 20212 was pushed, we lost (didn't dig enough to find out how)
the sorting feature for this route.

This is mainly because biblio+biblioitems filtering prevents the route
to use plain $c->objects->search, so it has baked a custom version of
it. In this case, it missed the call to the dbic_merge_sorting helper,
that takes care of reading the query parameters and inject the sorting
attributes to the resultset.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/acquisitions_orders.t
=> FAIL: Tests fail, sorting doesn't actually work
3. Apply this patch
4. Repeat 2
=> SUCCESS: Sorting works!
5. Test sorting the pending orders on parcel.tt
6. Sign off :-D

Sponsored-by: ByWater Solutions

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-02-04 14:21:35 -05:00
Jonathan Druart 7114dc2fb1 Bug 29914: Make check_cookie_auth compare the userid
check_cookie_auth is assuming that the user is authenticated if a cookie exists
and that the login/username exists in the DB.

So basically if you hit the login page, fill the login input with a
valid username, click "login"
=> A cookie will be generated, and the sessions table will contain a
line with this session id.
On the second hit, if the username is in the DB, it will be enough to be
considered authenticated.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-01-21 06:54:36 -05:00
Tomas Cohen Arazi 5ee42b6b60 Bug 29508: Make GET /patrons/:patron_id use Koha::Patrons->search_limited
This patch makes the route return 404 if the user is not allowed to see
the requested patron information.

To test:
1. Apply the regression tests
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: The code doesn't respect limits
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-01-07 06:35:54 -05:00
Tomas Cohen Arazi 521a5c41c5 Bug 29506: (follow-up) Adapt GET /patrons
This patch makes GET /patrons rely on this new behavior from the
objects.search helper.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-01-07 06:34:38 -05:00
Tomas Cohen Arazi 547c09ee6a Bug 29506: Make objects.search call search_limited if present
This patch makes objects.search implicitly update the passed
*$result_set* to use search_limited. This way no object leaks could
happen without noticing.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> FAIL: Tests fail because search_limited is not used
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! Results are correctly filtered based on userenv!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-01-07 06:34:38 -05:00
Tomas Cohen Arazi 7d38b15d65 Bug 29503: Make GET /patrons use Koha::Patrons->search_limited
This patch makes the controller method use Koha::Patrons->search_limited
so filters based on configuration and permissions apply when fetching
patrons.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Boo, you get more patrons than you should
3. Apply the patch
4. Repeat 2
=> SUCCESS: Yay! Things are filtered as expected (i.e. using
Koha::Patron->libraries_where_can_see_patrons)
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-01-07 06:32:38 -05:00
Tomas Cohen Arazi 339d199be7 Bug 29741: (follow-up) Make DELETE /patrons use the new validation method
This patch adapts the route so it uses the newly introduced
Koha::Patron->safe_to_delete method.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

Note: There's a trivial behavior change, in which the 'anonymous patron'
use case is caugh eariler than the ->delete call. I left the exception
catch block just in case, who knows :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
2022-01-07 06:27:56 -05:00
Tomas Cohen Arazi 0e23a87769 Bug 29018: Make DELETE /patrons/:patron_id check things
When the route was implemented, the checks were overlooked. This patch
adds checks for:
- Guarantees
- Debts
- Current checkouts

Any of those will block deletion, as it should.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Tests fail, the route misses checks
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! The three conditions prevent deletion!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2022-01-07 06:24:24 -05:00
Jonathan Druart eb017ee239 Bug 24850: Correctly handle suspended_until for PUT
if we are passing suspended_until to the body, it should be a rfc3339 (what the spec is expecting)
but $hold->suspend_until will be iso

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-11-03 15:40:52 +01:00
Jonathan Druart 5da73b604f Bug 24850: Fix api/v1/holds.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-11-03 15:40:52 +01:00
Tomas Cohen Arazi 8fdd52b17c Bug 29290: Add GET /biblios/:biblio_id/checkouts
This patch adds the required controller method.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 17:32:34 +02:00
Tomas Cohen Arazi 9ef23bb642 Bug 27358: Add GET /public/biblios/:biblio_id/items
This patch introduces a route to fetch items belonging to a biblio. It
is expected to return the 'public' representation of the Koha::Item
objects.

It is also enforcing the visibility rules, by using
Koha::Items->filter_by_visible_in_opac.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Test pass and they cover all the cases!
3. Try your favourite REST tool against the new route.
4. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 17:32:34 +02:00
Tomas Cohen Arazi 7647d51478 Bug 28948: Teach objects.search about public requests
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 17:32:34 +02:00
Tomas Cohen Arazi 9b2226a649 Bug 28948: Make is_public stashed on public routes
This patch makes the API authentication code stash the 'is_public' value
when public routes are hit.

This will be particularly useful to have $c->objects->search generically
pass this info down to the ->to_api method.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t
=> SUCCESS: Tests pass! When a public route is reached, the controller
has the 'is_public' value stashed
3. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 17:32:34 +02:00
Tomas Cohen Arazi e68f715340 Bug 17314: Implement /suggestions routes
This patch introduces routes to handle purchase suggestions, from the
staff POV.

Tests are added as well.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/suggestions.t
=> SUCCESS: Tests pass! And they are meaningful!
3. Play with your favourite REST tool (Postman?)
4. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 17:32:34 +02:00
Tomas Cohen Arazi 5b09620287 Bug 29272: (QA follow-up) Category setup overrides syspref
This patch removes an early short-circuit we had, which wasn't correct
regarding the OpacPasswordChange syspref. If a patron category is
allowed to change password, it overrides the syspref.

To test:
1. Tests still pass.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 12:16:37 +02:00
Tomas Cohen Arazi d91d813e5c Bug 29272: Make public password changing honour category constraints
This patch makes the public API routes validate
$user->category->effective_change_password before allowing the change.

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons_password.t
=> FAIL: Tests fail, it allows the first change instead of returning
         403.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-28 12:16:37 +02:00
Jonathan Druart 4390b7be04 Bug 28785: Adjust check_cookie_auth calls
The previous patch makes check_cookie_auth return the session instead of
$sessionID, so we are adjusting the different calls to prevent
confusion.
However they are mainly used to check the authentication status and
don't care about this second variable.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-18 11:28:41 +02:00
Tomas Cohen Arazi e85290a703 Bug 27947: (QA follow-up) Make parameters a hashref
This patch makes the Koha::ArticleRequest->cancel parameters into a
hashref. (Missing) tests are added for those parameters being set as
well.

Calls to ->cancel are updated.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Koha/ArticleRequest.t \
           t/db_dependent/api/v1/article_requests.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 10:09:54 +02:00
Tomas Cohen Arazi 9c4a1b19aa Bug 27947: (QA follow-up) Clarify permissions
Privileged routes need permissions defined. This patch adds the minimum
required permission until there are article request-specific permissions
in Koha: circulate: circulate_remaining_permissions

It is also clarified that interacting with an article request  from
another patron, but having your own patron_id in the path would return
404 instead of 403, as technically the resource (an article request from
the patron, identified.by the supplied id) doesn't exist.

Tests are tweaked.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 10:09:25 +02:00
Tomas Cohen Arazi dc79aa0f40 Bug 27947: (QA follow-up) Refactor routes
This patch refactors the route specs a bit, and also reorganizes code
for easier tracking.

Unused exceptions that were added earlier are removed for now.

A follow-up patch will add tests to this routes.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 10:09:25 +02:00
Agustin Moyano a10af06203 Bug 27947: Add cancellation reason to article request
This bug adds a cancellation reason authorised values to article requests

To test:
1. apply this patch
2. updatedatabase
3. in staff interface go to /cgi-bin/koha/admin/authorised_values.pl
CHECK => AR_CANCELLATION category should appears
4. place several article requests
5. in staff interface go to /cgi-bin/koha/circ/article-requests.pl
6. select multiple requests, or just one and cancel them
SUCCESS => a modal pops up offering to select a cancellation reason
CHECK => message_queue table has messages with cancellation reason included
7. repeat steps 4 to 6 but for /cgi-bin/koha/circ/request-article.pl
8. cancelling article requests from opac interface should work just as before

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-10-06 10:09:25 +02:00
Tomas Cohen Arazi 5e497a47a2 Bug 29032: Pre-load ILL backends to speed up response
Bug 22440 will rewrite the route and make it even more efficient by
prefetching the related data instead of performing several queries in
loops.

In the meantime, we can make this controller perform better with a
simple intervention: load backends once, and use the
$request->_backend() setter to pre-set it before using the objects.

To test:
1. Perform any usual ILL requests listing, try having several
=> FAIL: Notice it takes a weird amount of time to load
2. Apply this patch
3. Restart all
4. Repeat 1
=> SUCCESS: It feels fast enough!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-09-21 20:22:56 +02:00
Martin Renvoize b5a09e4654 Bug 22435: ->apply() should always use 'APPLY' for offset_type
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-08-04 14:06:43 +02:00
Jonathan Druart 9d6d641d1f Bug 17600: Standardize our EXPORT_OK
On bug 17591 we discovered that there was something weird going on with
the way we export and use subroutines/modules.
This patch tries to standardize our EXPORT to use EXPORT_OK only.

That way we will need to explicitely define the subroutine we want to
use from a module.

This patch is a squashed version of:
Bug 17600: After export.pl
Bug 17600: After perlimport
Bug 17600: Manual changes
Bug 17600: Other manual changes after second perlimports run
Bug 17600: Fix tests

And a lot of other manual changes.

export.pl is a dirty script that can be found on bug 17600.

"perlimport" is:
git clone https://github.com/oalders/App-perlimports.git
cd App-perlimports/
cpanm --installdeps .
export PERL5LIB="$PERL5LIB:/kohadevbox/koha/App-perlimports/lib"
find . \( -name "*.pl" -o -name "*.pm" \) -exec perl App-perlimports/script/perlimports --inplace-edit --no-preserve-unused --filename {} \;

The ideas of this patch are to:
* use EXPORT_OK instead of EXPORT
* perltidy the EXPORT_OK list
* remove '&' before the subroutine names
* remove some uneeded use statements
* explicitely import the subroutines we need within the controllers or
modules

Note that the private subroutines (starting with _) should not be
exported (and not used from outside of the module except from tests).

EXPORT vs EXPORT_OK (from
https://www.thegeekstuff.com/2010/06/perl-exporter-examples/)
"""
Export allows to export the functions and variables of modules to user’s namespace using the standard import method. This way, we don’t need to create the objects for the modules to access it’s members.

@EXPORT and @EXPORT_OK are the two main variables used during export operation.

@EXPORT contains list of symbols (subroutines and variables) of the module to be exported into the caller namespace.

@EXPORT_OK does export of symbols on demand basis.
"""

If this patch caused a conflict with a patch you wrote prior to its
push:
* Make sure you are not reintroducing a "use" statement that has been
removed
* "$subroutine" is not exported by the C4::$MODULE module
means that you need to add the subroutine to the @EXPORT_OK list
* Bareword "$subroutine" not allowed while "strict subs"
means that you didn't imported the subroutine from the module:
  - use $MODULE qw( $subroutine list );
You can also use the fully qualified namespace: C4::$MODULE::$subroutine

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-07-16 08:58:47 +02:00
Tomas Cohen Arazi 883ee12560 Bug 28588: (follow-up) Make the API use the new method
This patch adapts the controller method for resolving a return claim so
it uses the Koha::Checkouts::ReturnClaim method instead.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-23 15:09:56 +02:00
Tomas Cohen Arazi 7a908117dc Bug 28604: Prevent double encoding of MARC::Record::MiJ->to_mij output
This patch fixes a double-encoding issue with MiJ output.

Mojolicious' *text* renderer encodes the passed information according to
the request context. [1]

MARC::Record::MiJ->to_mij, conveniently encodes the string before
output [2].

This causes double encoding.

So the solution to this situation, is to use the *data* renderer, which
doesn't perform any encoding [3].

To test:
1. Apply the regression tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/biblios.t
=> FAIL: Tests contain diacritics and fail!
3. Have a record with diacritics
4. Try the API routes for fetching a biblio:
   $ curl --location --request GET 'http://localhost:8080/api/v1/public/biblios/144' \
          --header 'Accept: application/marc-in-json'
   (replace the record id with the one you've chosen)
=> FAIL: Boo, double encoding
5. Bonus point: you can try it on the non-public route, but you need
   more configuration boilerplate (basic auth, permissions). If you look
   at the fix, you will understand the tests cover it and no need to
   complicate yourself.
6. Apply this patch
7. Repeat 2
=> SUCCESS: Tests pass!
8. Repeat 4 (and maybe 5)
=> SUCCESS: No double encoding! Yay!
9. Sign off :-D

[1] https://metacpan.org/release/MRAMBERG/Convos-0.5/view/local/lib/perl5/Mojolicious/Guides/Rendering.pod#Rendering-text
[2] https://metacpan.org/dist/MARC-File-MiJ/source/lib/MARC/Record/MiJ.pm#L111
[3] https://metacpan.org/release/MRAMBERG/Convos-0.5/view/local/lib/perl5/Mojolicious/Guides/Rendering.pod#Rendering-data

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-23 15:09:56 +02:00
Jonathan Druart 58db137c0a Bug 28271: Add the ability to set a new lost status when a claim is resolved
When a claim is resolved the librarian should be able to edit the lost
status of the item.

It is also letting a way out when BlockReturnOfLostItems is set (the
lost status is stuck on ClaimReturnedLostValue as we remove the
option to edit itemlost on the item edit form).

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-21 10:50:09 +02:00
Tomas Cohen Arazi 639aa95a50 Bug 28586: (follow-up) updated_by should be set
This patch makes the route set the 'updated_by' attribute as well, when
resolving a return claim through the API.

Tests are added for this behavior.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/return_claims.t
=> SUCCESS: Tests pass! updated_by is set correctly!
3. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-21 10:09:58 +02:00
Tomas Cohen Arazi bbe424dcfb Bug 27931: Add GET /items/:item_id/pickup_locations
This patch adds the mentioned route. It does so by:
- Adding the new path in paths.json
- Adding the full route spec in items.json
- Adds a controller method that takes care of the task

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/items.t
=> SUCCESS: Tests pass!
3. Play with your favourite REST tool. Pay special care to
   the AllowHoldPolicyOverride syspref and the expected behaviors.
=> SUCCESS: All works as expected
4. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-06-14 17:35:19 +02:00
Tomas Cohen Arazi a554b5a545 Bug 28369: Set additionalProperties: false and fix things
This patch performs a cleanup on the holds routes:
- It adds additionalProperties: false to all missing routes
- It syncs the spec with the guidelines
- additionalProperties: false highlighted some attributes read in the
  controlled weren't declared on the spec, adding them
- PUT /holds/:hold_id is moved to PATCH, as it makes more sense and
  gathered opinions lean towards that

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Lots of things are wrong with the API
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-25 09:28:18 +02:00
Tomas Cohen Arazi cfcc34fde7 Bug 28424: Fix patron credits route (POST)
This patch makes the route follow the coding guidelines and thus:
- Return the generated object
- Use the status code 201 for the response

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons_accounts.t
=> FAIL: Tests fail. The reponse body and status is incorrect
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-25 09:28:17 +02:00
Tomas Cohen Arazi d1915eb24b Bug 28189: (QA folloq-up) Fix fallback case
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-12 17:39:36 +02:00
Tomas Cohen Arazi 3c16e0110e Bug 28189: Move swagger file to YAML format
This patch changes the base OpenAPI file (swagger.json) into YAML. The
motivation for this, is adding more documentation, in Markdown.

JSON doesn't accept multiline strings, so this seems like a good move
for readability.

To test:
1. Verify your API is functional
2. Apply this patch
3. Repeat 1
=> SUCCESS: No changes, really
4. Point your browser to /api/v1/.html
=> SUCCESS: Some nicely formatted description of the API can be seen. It
includes information about x-koha-library.
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-12 17:39:36 +02:00
Martin Renvoize 102deb8cd9 Bug 28254: (QA follow-up) override -> force
After some discussion we decided that the parameter should be called
'force' for pushign through an action against policy. (as per the
Transfers work).

We decided to stick to 'override' at the API level however as it
highlights the 'human interaction' element of the call.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:08 +02:00
Tomas Cohen Arazi 01b9367625 Bug 28254: Make PUT /holds/:hold_id/pickup_location honour x-koha-override
This patch adds a new parameter (x-koha-override header) to the route,
and makes the controller pass this information (override requested) when
the AllowHoldPolicyOverride syspref is set, to
Koha::Hold->set_pickup_location.

This way, under certain conditions, we can be sure the update wont'
fail.

To test:
1. Apply this patchset
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:08 +02:00
Tomas Cohen Arazi ede8290c5d Bug 27898: Make PUT /holds/:hold_id accept overrides
This patch makes the route handle overriding pickup locations by passing
the 'x-koha-override' header (set to 'any').

To test:
1. Apply the unit tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail, because things cannot be overridden ATM.
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:04 +02:00
Tomas Cohen Arazi e5d5fbdd3e Bug 27797: (QA follow-up) Pickup locations can be overridden too
This patch adds regression tests for overridding pickup locations, which
was inadvertedly not covered by the original dev. What this does, is
moving the $can_override variable definition above, and avoid returning
400 if conditions are not met for the passed pickup_library_id, when
$can_override is true.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:04 +02:00
Nick Clemens 01209c228d Bug 27797: (QA follow-up) Additional tests and operator change
I added a few tests to cover additional cases and found the operator precedence
was failing, switched 'and' to '&&'

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:04 +02:00
Tomas Cohen Arazi 098e93e7cc Bug 27797: Make POST /holds use the stashed koha.overrides
Bug 26181 introduced a way to override behavior through a new header,
x-koha-override. And bug 27760 introduced a generic approach to handle
x-koha-override.

This patchset makes the POST /api/v1/holds route rely on this new way of
handling x-koha-override instead of doing it manually.

The header is added to the spec as a parameter.

Note: the header should be defined as:

    "type": "array",
    "collectionFormat": "csv",
    "items": {
        "type": "string",
        "enum": [
            "any"
        ]
    }

but the versions of JSON::Validator we use have lots of bugs related to
header type coercion, so it just doesn't work. The changelog for
JSON::Validator is fairly elocuent about it.

The override now takes a list of valid keywords (right now only 'any'
but it would be fairly straight-forward to add a fairly granular set of
options based on each possible AddReserve failure.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
4. Sign off!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:03 +02:00
Tomas Cohen Arazi c80d4655cc Bug 27760: (QA follow-up) Improve POD
This patch improves the POD for the added helper, and also adds
references to the JSON::Validator issue that explains the current
implementation. A FIXME is added for when we upgrade our used
JSON::Validator version.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:02 +02:00
Jonathan Druart 25b08f9989 Bug 27760: Improve readabiliity using map
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:02 +02:00
Tomas Cohen Arazi 91ce8a098c Bug 27760: Make authenticate_api_request call stash_overrides
By making this method call the helper, we make sure the overrides
hashref is available to any controller to make us of it.

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t \
           t/Koha/REST/Plugin/Query.t
=> SUCCESS: Tests pass! The helper works as expected and the
authenticate_api_request method invokes it!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:02 +02:00
Tomas Cohen Arazi c2cc6b401a Bug 27760: Add stash_overrides helper
This patch adds a simple helper that reads the x-koha-override request
header, and processes it to stash a hashref with the passed overrides.

No check on the overrides themselves is done, as they should be
validated using the OpenAPI spec.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t
=> SUCCESS: Tests pass!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-05-04 14:22:02 +02:00
Tomas Cohen Arazi 2db98432d8 Bug 27932: Add GET /biblios/:biblio_id/pickup_locations route
This patch adds the mentioned route. It does so by:
- Adding the new path in paths.json
- Adding the full route spec in biblios.json
- Adds a controller method that takes care of the task

To test:
1. Apply this patches
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/biblios.t
=> SUCCESS: Tests pass!
3. Play with your favourite REST tool. Pay special care to
   the AllowHoldPolicyOverride syspref and the expected behaviors.
=> SUCCESS: All works as expected
4. Sign off :-D

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-30 17:19:01 +02:00
Martin Renvoize fb3a67f21b Bug 27995: Update return for Koha::Account::Line->apply
This patch updates the return value of Koha::Account::Line->apply to return the
updated Koha::Account::Line object with Koha::Object::Messages embedded
for the results of any triggered renewals.

Available credit, the former return value, is still availabe via the
object directly as `amountoutstanding`.

Koha::REST::V1::Patrons::Account->add_credit has been updated to reflect
the change. No other area's of Koha relied upon the return value.

This patch also fixes a bug whereby if you passed a list of debits with
an outstanding debit larger than the outstanding credit then you could
end up with superflous offest lines with zero amounts that was
highlighted by the improved unit tests in
t/db_dependent/Koha/Account/Line.t

Test plan
1/ Run t/db_dependent/Koha/Account/Line.t
2/ Run t/db_dependent/api/v1/patrons_accounts.t
3/ If both tests pass then signoff

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-27 16:23:49 +02:00
Tomas Cohen Arazi 79087e0674 Bug 28002: Add extended_attributes support to POST /patrons
This patch adds support for the 'extended_attributes' parameter in the
route for adding a patron. It relies on
Koha::Patron->extended_attributes for the tests. Exceptions are catch
and the whole operation is rolled back.

I chose to handle each exception on its own if branch, with bug 28020 in
mind.

To test:
1. Apply this patchset
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCES: Tests pass!
3. Check they cover all the exception situations!
=> SUCCESS: They do!
4. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-22 14:50:12 +02:00
Tomas Cohen Arazi 6d1b7ca3a0 Bug 28157: Add handling for the x-koha-library header
This patch introduces a new header handling. The key idea is that on
Koha's base classes there's broad usage of C4::Context->userenv to
determine the current library and make decisions based on that.

API requests, on the other hand, might not be tied to sessions
(stateless) which is the way current library is retrieved. So we need a
way to properly specify what library the request is trying to act as
coming from.

To test:
1. Apply this patchset
2. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t
=> SUCCESS: Tests pass!
3- Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-22 14:50:12 +02:00
Tomas Cohen Arazi 49d764e413 Bug 23666: (follow-up) Adjust to new exceptions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2021-04-22 14:50:12 +02:00