This patch introduces tests for the behaviour change (400 => 404) and
also adds tests for untested (error) behaviours. Notably, the 'Biblio
not found' problematic return value couldn't be triggered, because the
OpenAPI plugin returns a 500 because the response was malformed
(expected { error => 'message' } and it was returning a string).
For the above reason, running the regression tests will show a 500...
instead of the expected 400.
I covered the rest of the codepaths that trigger errors to have full
coverage.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/clubs_holds.t
=> FAIL: Tests fail loudly
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>
This patch creates a Koha::Cash::Register::Cashup(s) class pair which
subclass Koha::Cash::Register::Action(s) and moves the cashup specific
code into these new classes to improve code separation.
We then introduce API routes based on these classes to allow fetching
a list of cashups associated to a cash register and a full cashup with
emeddable summary for individual cashups.
Test plan
1/ Run the updated unit tests.
t/db_dependent/Koha/Cash/Register/Action.t
t/db_dependent/Koha/Cash/Register/Cashup.t
2/ Run the incuded api tests.
t/db_dependent/api/v1/cashups.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
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>
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>
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>
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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch puts the tests in line with the current codebase. Some bits
are passed through perltidy to aid readability...
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/api/v1/clubs_holds.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
=> SUCCESS: Tests look more like the rest of the API tests
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
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>
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>
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>
This patchset did a copy and paste of the objects->search helper inside
the controller, to handle a very special case that wasn't worth
generalizing: the fact that the biblio object (that this patchset makes
embeddable) is actually built from two tables, and we want to filter by
fields in both tables.
This requires us to translate:
- Any filter reference to fields in biblioitems we allow to search on, through
an allow list.
- For filtering to work, we need to tweak the stashed 'embed' object so
the prefetch is done correctly
This patchset also adds a new query parameter: only_active. It is used
to only request active orders (see the spec for more details).
This patch adds tests for:
- Filtering by a column in bilbioitems (this implies correctly
prefetching the bilbio => biblioitems relation, and translating the
filtering parameter.
- Using the only_active query parameter to retrieve only active orders
To test:
1. Apply this patchset
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/acquisitions_orders.t
=> SUCCESS: Tests pass!
3. Verify the tests check for the explained behaviours.
=> SUCCESS: They do!
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: 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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If the account of the patron who created a claim is removed, then the
claim won't be resolvable.
Trying to resolve the claim will result in a 500:
[2020/12/21 10:05:55] [ERROR] PUT /api/v1/return_claims/1/resolve: unhandled exception (Koha::Exceptions::Checkouts::ReturnClaims::NoCreatedBy)<<[created_by is mandatory]>> Koha::REST::Plugin::Exceptions::__ANON
We should not check if created_by exist when we update an existing hold,
only when we create it.
Test plan:
- Login with patron A
- set a value to syspref ClaimReturnedLostValue
- Check an item out
- Claim returned it
- Login with patron B
- Delete patron A
- Resolve the claim
- ERROR in api-error.log an UI never confirming the resolution
- apply patch & restart services
- Resolve the claim
- it works
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The GET /pickup_locations route is requesting the whole reserveforothers
permission whereas only the subpermission place_holds is needed.
Test plan:
0. Don't apply this patch
1. Set the subpermission place_holds but modify_holds_priority
2. Edit a hold and click the pickup library dropdown list
3. You get a JS alert and log displays
GET /api/v1/app.pl/api/v1/holds/5/pickup_locations
403 Forbidden
4. Apply this patch
5. Reload the page, click the dropdown list, modify the pickup location
and save
=> Success!
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>
This patch adds a check for valid pickup location to the POST /holds
route. A 400 code is returned if the supplied pickup library is not
valid.
To test:
1. Apply this patch
2. Run;
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a test for valid pickup locations when updating a hold
through the API.
Tests are adjusted to reflect this change.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Abbey Holt <aholt@dubuque.lib.ia.us>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds tests for the change. It also simplifies the delete()
method structure a bit. It fixes the error 500 the tests were raising.
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Tests fail!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass! The new behaviour (code 403) is tested!
4. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The method call set_waiting(1) was renamed to be set_transfer()
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces API routes for handling SMTP servers.
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>
.pm must not have -x
.t must have -x
.pl must have -x
Test plan:
Apply only the first patch, run the tests and confirm that the failures
make sense
Apply this patch and confirm that the test now returns green
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When manager_id is null/undef the API returns error code 500.
To test:
1) Notice failure when running
prove t/db_dependent/api/v1/patrons_accounts.t
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The API was only catching the primary email change case, but we need to
catch email, emailpro and B_email.
We were also not accounting for any of the emails (on PUT or from the
DB) being undefined.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Most of the time the tests are failing because the item is not created
correctly (missing biblio and/or biblioitem).
The usual error is:
t/db_dependent/selenium/regressions.t ..... 5/5 Can't call method "leader" on an undefined value at /kohadevbox/koha/Koha/SearchEngine/Elasticsearch.pm line 534.
In this patch we are making sure $builder->build({ source => 'Item' })
is replace with $builder->build_sample_item
Test plan:
Turn on Elastic and confirm that all the tests pass!
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds trivial tests for the new error (400) raised when the
passed patron_id doesn't exist.
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
2. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Currently, the main table of ILL requests will display all ILL
requests in the database, regardless of their status. For libraries
with active ILL this quickly leads to a lot of requests being displayed,
and the main page of the ILL module taking a long time to load. This
patch proposes to fix this by introducing the ILLHiddenRequestStatuses
syspref, which can take a pipe-separated list of ILL statuses that
will be hidden from view in the ILL module. This means that the
only way to find a hidden request will be through a report.
To test:
- Apply the patch and make sure the atomic database update is run
- Make sure you have a few ILL requests, with at least two different
statuses
- Check that all requests are still displayed in the main table of
ILL requests
- Add one of the statuses* you have in your database to the
ILLHiddenRequestStatuses syspref, reload the ILL module frontpage
and verify that requests with the given status are not displayed
- Change the syspref to another status and verify requests with
that status are now hidden
- Change the syspref to hold both statuses, separated by the pipe
symbol (e.g.: A|B). Verify that no requests with the given
statuses are now displayed
- Run the ILL REST API tests, e.g.:
$ sudo koha-shell -c "prove t/db_dependent/api/v1/illrequests.t" kohadev
* = The ILLHiddenRequestStatuses syspref should hold status codes, like
"REQ" and "NEW", not their human readable counterparts.
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixes the libraries tests, that expected the old behaviour
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/api/v1/libraries.t
=> FAIL: Tests fail
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a missing transaction wrapping one of the subtests. This
is probably an oversight when refactoring this file.
To test:
1. Open the patron categories page
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/oauth.t
=> SUCCESS: Tests pass
3. Reload the patron categories page
=> FAIL: There's random data in there
4. Apply this patch and repeat 2
5. Reload the patron categories page
=> SUCCESS: The random data from the previous tests is still there, but
no new random data
=> SUCCESS: Tests still pass
6. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds regression tests for the different authentication
mechanisms Koha supports. It highlights the fact that Koha expects
userenv to be set on authentication.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
The API related tests still don't pass with the previous modifications.
They pass on D10 but fail on U18 and I did not manage to find where the
problems come from.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I'm not sure what we need to test here, besides we get results. I'm
pretty sure we shouldn't allow a plain GET return all objects because it
could cause a DOS very easily. But it certainly belongs to a separate
bug/discussion.
In this case, I put a constraint on the items domain (by using
_per_page=10 in the query params) so it won't happen that under certain
conditions the user agent used by Test::Mojo time outs.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The original development started before the changes we introduced in the guidelines in late 2019, and the major code changes that took place in January 2020.
- 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
Those are fixed by this patch.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/advanced_editor_macros.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
