This patch improves the semantics of the update routine to more clearly
separate the 'fields' we're modifying from the 'options' we wish to
apply to the modification.
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
# Failed test 'POD test for Koha/Acquisition/Orders.pm'
# at /usr/share/perl5/Test/Pod.pm line 187.
# Koha/Acquisition/Orders.pm (43): Non-ASCII character seen before =encoding in 'uivalent'. Assuming UTF-8
xt/author/podcorrectness.t .. 622/829 # Looks like you failed 1 test of 829.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
No idea what's happening with the current_timestamp, but previous change
was wrong.
Signed-off-by: Angela O'Connor Desmond <angela.oconnordesmond@staff.ittralee.ie>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
So far we only record the number of claims and the date of the last
claim, in the aqorders table.
To keep track of the different claim dates, this patchset is going to
make the following DB changes:
* Create a new table 'aqorders_claims' (id, ordernumber, claimed_on)
* Remove the two columns from the aqorders table: claims_count and
claimed_date
This will allow to display the different claim dates where needed: on
the late orders page, and the basket page.
To avoid additional fetches of Koha::Acquisition::Orders, GetLateOrders
has been moved to Koha::Acquisition::Orders->filter_by_late
That way we are going to add consistency, robustness, and cover the
feature with new tests.
Test plan:
0/ Create a bunch of new orders. Make sure they are from different
vendor (with different delivery time).
1/ Go to the late orders page and claim some orders
2/ Reclaim some of those orders
3/ Confirm that you can see the different claim dates for a given orders
(the history of the late orders claims is kept and displayed)
4/ Bonus point: Regression tests:
a. Modify the closedate of the basket in the database. That
will allow you to make sure the patch set did not introduce regressions.
It would be good to test the different filters on the late orders page:
* delay
* Estimated delivery date from/to
* Vendor
b. Confirm that the subtotal and the total values from the late orders
page is correct.
c. Test the update database entry: do not apply these patches, claims
some orders against master. Apply the patch, execute the update DB entry
then confirm that the number of claims is correct (note that the dates
will not as it is not possible to guess them).
QA note: the branchcode parameter has been removed from filter_by_late.
At first glance it seems that it was not used.
Sponsored-by: Cork Institute of Technology
Signed-off-by: Angela O'Connor Desmond <angela.oconnordesmond@staff.ittralee.ie>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Sponsored-by: Cork Institute of Technology
Signed-off-by: Angela O'Connor Desmond <angela.oconnordesmond@staff.ittralee.ie>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Sponsored-by: Cork Institute of Technology
Signed-off-by: Angela O'Connor Desmond <angela.oconnordesmond@staff.ittralee.ie>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch moves the error messages to translatable strings
Also includes capitalization fixes and POD adjustment
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - Enable AdvancedCatalogingEditor
2 - Add some macros (they don't need to be valid, just have content)
3 - Apply patches
4 - updatedatabase
5 - Set user to have 'create_shared_macros' and 'delete_shared_macros'
6 - Load the advanced editor (Cataloging->Advanced editor)
7 - Click on 'Macros'
8 - Previous macros should not show, but you should have a 'Convert' button
9 - Convert old macros and confirm they show
10 - Edit the macros, changing content and the public checkbox, confirm 'Saved' shows in the top right of editor when updating
11 - Have at least on valid macro and run it, e.g.:
new 100=Testing this out
12 - Run the macro, confirm it runs
13 - Try a macro with gibberish, confirm there is an error when running
14 - Ensure you have a few macros marked public
15 - In a private browser window sign in as a patron with neither shared macro permission
16 - Confirm the public macros load, but cannot be edited
17 - Grant create_shared_macros permission to this patron
18 - Reload editor, they should now be able to edit shared macros
19 - Confirm they cannot delete shared macros
20 - Grant delete_shared_macros permission
21 - Reload editor
22 - Confirm they can now delete shared macros
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Heather Hernandez <Heather_Hernandez@nps.gov>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch makes the return values for 'current_item_level_holds'
consistent: they will always be a Koha::Holds iterator.
To test:
1. Apply the regression tests
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Acquisition/Order.t
=> FAIL: It doesn't work as expected, cannot call ->count on undef, etc
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: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
In case we already started to iterate.
This fixes a failing tests in t/db_dependent/Koha/Biblios.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
If we call Koha::Libraries->delete but Koha::Library->delete exists
(ie. Koha::Object->delete is overridden), then we Koha::Objects->delete
will be called and the overridden will not be executed.
This patch suggests to test if the method is overridden (using
Class::Inspector->function_exists). If so we loop on the different
objects of the set in a transaction and call the overridden ->delete method.
Existing tests widely cover this change.
t/db_dependent/Koha/Objects.t
subtest 'Return same values as DBIx::Class' => sub {
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
In OO we would usually instantiate the class and then interact with it
like in:
my $rs = Koha::Patrons->new;
$rs->empty;
Koha's practice is to call things like Koha::Patrons->search without
instantiating the Koha::Patrons class. To keep consistency, this patch
instantiates the resultset object on behalf of the caller if required.
To test:
1. Apply the tests patch
2. Run:
$ kshell
$k prove t/db_dependent/Koha/Objects.t
=> FAIL: Tests fail because it is expected to have the class
instantiated
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass, instantiation happens implicitly.
5. 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: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adds a new ->empty method to Koha::Objects, that can be used
to make the underlying DBIC resultset empty. This way, we can have
consistency in our method's return values without the need to build
a query that we know in advance that will be empty. No need to hit the
DB at all.
To test:
1. Apply this patches
2. Notice the tests cover what is expected
3. Run:
$ kshell
k$ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests pass! Yay!
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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch makes the route support requesting (through the Accept
header) the MARC record to be output as formatted text as in
$record->as_formatted.
To test:
1. Apply the unit tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/biblios.t
=> FAIL: Tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Try the route with your favourite API testing tool (Postman?)
=> SUCCESS: Accept: text/plain returns the expected results
=> SUCCESS: Wrong Accept header returns a list of valid formats, and
includes 'text/plain'.
6. 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: 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>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To match the coding guideline SQL12.
We do this for consistency.
Note that we do not need an update DB as BOOLEAN is an alias for
TINYINT(1).
The is_boolean flags have been added to the schema.
Test plan:
1. `dbic` should run correctly
2. kohastructure.sql should be syntaxicaly correct
3. `git grep -i boolean installer/data/mysql/kohastructure.sql`
should not return relevant occurrences.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor
2 - Reindex elasticsearch
3 - Check the ES count on the about page
4 - Check the count in the DB (SELECT count(*) FROM biblio)
5 - They don't match!
6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v
7 - No errors indicated
8 - Apply patch
9 - perl misc/search_tools/rebuild_elastic_search.pl -v
10 - You should be notified of an error
11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v
12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason
13 - perl misc/search_tools/rebuild_elastic_search.pl
14 - No output
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - Enable SMS using the Email driver
2 - Add an SMS provider under admimistration
3 - Add an SMS number and provider to a patron
4 - Note on the patron details you don't see the provider :-(
5 - Apply patch
6 - Reload
7 - Note you do see the provider :-)
8 - prove t/db_dependent/Koha/Patrons.t
9 - Sign off!
Signed-off-by: Lisette <lisetteslatah@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adapts the spec and the controllers so existing routes return
204 and an empty response body when a successful deletion happens.
Right now we have a coding guideline but haven't adapted the existing
routes.
To test:
1. Apply the regression tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/*.t
=> FAIL: Some tests fail
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
If we catch a Koha::Exception-derived exception, the log is put in a
single line. If the code 'dies' then a newline character is appended to
the string. This patch chomps it so it displays in a single line.
To test:
1. Tweak Koha::REST::V1::Cities::list in the try block so it dies before
render
2. Restart plack and try the original test plan
=> FAIL: Notice two lines are logged
3. Apply this patch
4. Repeat 2
=> SUCCESS: Only one line in the logs
5. Verify rendering a Koha::Exception works as well:
Koha::Exceptions::Exception->throw("Nada!");
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This simple patch removes 'just in case' handling of specific exceptions
and makes the current routes controllers use the unhandled_exception helper.
Most possible exceptions are already catch by our tools (Koha::Object,
etc) and the existing code is not looking for known possible exceptions
but has just been copied and pasted since our beginings.
Anytime a situation in which an unhandled exception is caught, we (the
devs) should report it and specific exception handling discussed and
solved. But this has just been useless scaffolding so far.
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1.
=> SUCCESS: Tests still pass
4. Sign off :-D
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adds Koha::Logger as the default logger for the API, and
introduces a new helper plugin that takes care of handling the unhandled
exceptions. Basically, with this we would write something like this in
our controller methods:
try {
...
}
catch {
if ( know_exception ) {
handle_known_exception($_);
}
$c->unhandled_exception($_);
}
Without this, we end up adding more and more handling 'just in case'.
To test:
1. Edit the Koha/REST/V1/Cities.pm 'list' method adding
die("Nada"); before the render step.
2. Restart plack and try the endpoint
=> FAIL: A generic error is displayed, and no traces of the original
problem are found on the logs.
3. Apply this patches, make sure your instance's log4perl has the
introduced lines for API with the right path.
4. Repeat 2
=> SUCCESS: The message is still generic, but you see something is
logged in /var/log/koha/kohadev/api-error.log
5. Change die("Nada"); for a real exception like:
use Koha::Exceptions;
Koha::Exceptions::DuplicateObject->throw("Nada");
6. Repeat 2.
=> SUCCESS: The message is generic, but a meaningful text is added to
the logs.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch correct all cases of return undef to instead return an empty
resultset.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
- save and run this sql query in reports: select sum(if(issues is null,1,0)),sum(if(issues=0,1,0)) from items
- you should see a lot of nulls and no zeros
- apply patch
- updatedatabase
- re-run your query and see that your nulls have changed to zeros
- create a new item
- rerun your query and see your new item is counted in the zeros, not the nulls
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch introduces a check on the authenticate_api_request method for
the RESTPublicAnonymousRequests system preference. If disabled,
anonymous requests get rejected.
The idea is to replicate the homologous OpacPublic system preference
behaviour.
To test:
1. Apply the Unit tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/auth_authenticate_api_request.t
=> FAIL: Tests fail, 200 is answered instead of 401 on the route.
3. Apply this patch
4. Repeat 2.
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adds a route to get bibliographic records without privileged
access. This needs to match the OPAC expected behaviour.
Some things were considered on the implementation:
- The ViewPolicy filter that hides/shows things based on the frameworks
needded to be used, as in the OPAC.
- OpacHiddenItems and OpacHiddenItemsExceptions need to be considered
for hiding records the same way the OPAC is expected to.
- Avoid using OpacHiddenItemsExceptions, but rely on the patron category
instead (use Koha::Patron::Category->override_hidden_items abstraction
is used instead so it should keep working once 22547 is moved
forward).
- Tests should cover all the use cases:
* logged in user
* anonymous user
* logged in with category that overrides
* logged in with category that doesn't override
This is all implemented on the tests.
To test:
1. Apply the tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/biblios.t
=> FAIL: Route not implemented
3. Apply the rest of the patches
4. Repeat 2
=> SUCCESS: Tests pass!
5. Try it with your favourite API tool (Postman?)
6. Sign off :-D
Note: please notice there isn't a default fallback behaviour for when
you don't specify the Accept header, so testing this on a regular
browser will just print the accepted mime types instead of the record
itself.
To test this with a tool (like Postman) you should enable
RESTBasicAuthe and make the tool use Basic authentication with valid
credentials. And you need to specify any of the following strings on the
Accept header:
- application/marcxml+xml
- application/marc-in-json
- application/marc
- text/plain
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I don't understand how that could be useful. We do not want to test if
the logfile is writable every time we log something!
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We do not want to fail silently for the logger.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The table borrower_modifications has no FK constraint on the borrowernumber
and will remain untouched when the patron is deleted.
If the borrowernumber doesn't exist in the database, the modification entry is no longer visible in Koha.
The problem is that this table is used for the borrower modifications and the self-registration features.
So far borrowernumber is the PK (int(11) NOT NULL DEFAULT '0'), for the self-registration feature we can have borrowernumber that is not defined (0 is used)
Ideally we would like to have borrowernumber a DEFAULT NULL, and use NULL for self-reg, but then we will loose the PK (PK cannot be NULL).
As we cannot keep the correct constraints at DB level anyway, we will need to handle consistency at code-level.
Test plan:
Create a new patron
Do some modification at the OPAC
Delete the patron
Confirm that the modifications as been removed (directly in DB)
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Tomas Cohen Arazi
Martin Renvoize
Jonathan Druart
Nick Clemens
Julian Maurice
Andrew Fuerste-Henry