In sub merge the Koha::Authority object was already created, so it was kind
of silly to recreate it each time with new.
In Koha::Authority, the ControlledIndicators object can be saved in the
object too for reuse during its life time. Similarly, we prevent repeated
lookups for the reporting tag (via authority type).
Test plan:
[1] Run t/db_dependent/Authority/Merge.t
[2] Run t/db_dependent/Koha/Authorities.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Limited benchmarking shows me a reduction of 11% on Merge.t and a
reduction of 4% on Authorities.t when comparing with and without this
last patch. The larger impact on Merge.t could be expected.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch does:
[1] Adds Koha::Authority->controlled_indicators (with a test).
[2] Adds a call to controlled_indicators in AuthoritiesMarc::merge.
Unit test Merge.t is extended too.
[3] Simplifies the code in authorities/blinddetail-biblio-search.pl by
calling controlled_indicators.
Test plan:
[1] Run t/db_dependent/Koha/Authorities.t
[2] Run t/db_dependent/Authority/Merge.t
[3] Steps 3 to 7 for MARC21:
Create a PERSO_NAME authority with 008/11=r and ind1=3
[4] Edit a biblio and add a 600 linked to the new authority.
[5] Verify that the biblio has ind1==3 and ind2==7 and $2==aat.
(If $2 is not visible, check the metadata in biblio_metadata.)
[6] Edit the PERSO_NAME authority and change 008/11 to '|' (bar).
[7] Verify that merge updated your biblio record: $ind2==4 and $2 gone.
[8] UNIMARC: Follow the pattern from steps 3 to 7.
Create authority, link it in a biblio, check indicators (they should
be copied both). Edit authority, change indicators and verify the
merge results in the biblio record.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested PERSO_NAME and UNIF_TITLE.
For UNIF_TITLE the second authority indicator is copied to ind1 or ind2,
depending on the biblio tag involved.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Apart from adding tests for this sub, we make the following change
too and support it with a test.
The fallback to $code in the construction
$thes_mapping{ $code } // $code // '4'
is removed by this patch. When $code is not in $thes_mapping, we cannot
assume that it is a valid code for a biblio indicator. In this case it is
more safe to mark it as 4 (Source not specified).
Test plan:
Run t/Koha/Authority/ControlledIndicators.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Original patch from Janusz Kaczmarek on November 24, 2017.
Amended by Marcel de Rooy on February 6, 2018.
Code moved from AuthoritiesMarc.pm to ControlledIndicators.pm.
Special attention has been paid to the proper application of 008/11
while controlling 6XX in MARC 21, specially if 008/11 =~ /[rsz]/
(and if it is 'z' and 040 $f is defined).
Test plan:
See next patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The construction $thes_mapping{ $code } // $code // '4' will still get
some attention on a follow-up.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This module reads its settings from pref AuthorityControlledIndicators.
The get method allows you to retrieve indicator values for specific
biblio fields.
Test plan:
Run t/Koha/Authority/ControlledIndicators.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: Added a test to show that "ind1:" returns empty string.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Changes in MarcTagStructure.pm
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Found this regression when working on other patches:
If you edit a patron and blank the userid field, it will be regenerated
with an incremented value (firstname.surname will be firstname.surname1)
This is because we use a non-existing patron and ->in_storage in
has_valid_userid is always false.
The trick here is to backup the value, generate the userid, then reset
userid to the previous value.
As the POD says, it will be fix later, when AddMember and ModMember will
be replaced with Koha::Patron->store
Signed-off-by: Josef Moravec <josef.moravec@gmail.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: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
C4::Members did not "use warnings;"
Signed-off-by: Josef Moravec <josef.moravec@gmail.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: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We previously prove that the method and the subroutine were equivalent,
we know update the controller calls.
Test plan:
- Add and update a patron with different variations of userid
(automatically generated or not)
- Import patrons with and without userid, as well as with existing
userid
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Reuse how C4::Members::Check_Userid works and adapt it to write
Koha::Patron->check_userid
Adapt the tests to use this new method.
The tests still pass, we can adapt the different callers
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It should also be noted that
/etc/apache2/sites-enabled/{configuration file}
should also contain:
RequestHeader add X-Koha-SetEnv "TZ {timezone}"
Where {timezone} is the desired instance timezone.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Most of Koha depends on the local timezone of the server, except for Koha::Database which support an ENV override with the key TZ.
We should take this a step further. We should not only accept the TZ environment variable for all of Koha, we should really be able to set the timezone in the koha conf file as well so we don't have to pass that environment variable to things like cronjobs and one-off scripts.
Test Plan:
1) Apply this patch
2) Set a timzone in your koha_conf file, that is *not* your local time zone
Available timzones are listed here: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
3) Restart apache/plack
4) Perform some actions, check the timestamps in the database and in the
html output, note they are for the set timezone and not the local
timezone.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Stolen from bug 20244
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This option in combination with append seems to add null entries in the indexes
for fields that don't exist. These fields then affect sorting. Removing it solves
the issue
To test:
1 - Have an ES setup
2 - Catalog and index some records
3 - View a record in ES:
curl 'localhost:9200/koha_kohadev_biblios/data/4?pretty=yes'
4 - Note there are 'null' entries in the indexes, save this result for
comparison
5 - Apply patch
6 - Reindex, amking sure to delete existing indexes
perl misc/search_tools/rebuild_elastic_search.pl -d
7 - View the record again
8 - Note the null entries are removed
9 - Note indices are otherwise intact
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Replaced with distinct.
Fix for:
'koha_kohadev.me.shelfname' isn't in GROUP BY
t/db_dependent/Virtualshelves.t
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
A number of Koha plugins have been written that enhance Koha's public catalog. These plugins often make due to adding css and javascript to the various opac system preferences. It would be nice if the plugin system had hooks so plugin developers could add code the the head block and the area where we include javascript in the opac template.
Test Plan:
1) Apply this patch
2) Download and install the Kitchen Sink plugin ( v2.1.12 or later )
https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/download/v2.1.12/koha-plugin-kitchen-sink-v2.1.12.kpz
3) Install the plugin
4) Restart all the things if you can ( restart_all if you are using kohadevbox )
This will ensure the plugin takes effect right away, it should be
necessary but it won't hurt anything!
5) Load the opac, notice you get an alert message and the background
for your opac is now orange ( assuming you've not customized the
opac in any way )
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Caused by
commit 04aea91de0
Bug 15685: (QA follow-up) Address QA issues
->find is called on Koha::Object instead of the set class (Koha::Objects)
and raises the following error:
Can't use string ("Koha::Acquisition::Basket") as a HASH ref while
"strict refs" in use at /usr/share/koha/lib/Koha/Object.pm line 275.
This patch also makes sure $basketno refers to an existing basket in DB
I cannot provide a test plan, I have no idea how this code is used.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Jenkins fails (run 287) on a test in t/db_dependent/Letters/TemplateToolkit.t:
With the historical syntax:
# Your request for an article from tQYRS (c3Av58O0P5xkkIGu) has been canceled for the following reason:
With the TT syntax:
# Your request for an article from tQYRS_ (c3Av58O0P5xkkIGu) has been canceled for the following reason:
The last character of the biblio's title has been removed because it's a punctuation character.
It comes from: C4::Letters::_parseletter
893 $val =~ s/\p{P}$// if $val && $table=~/biblio/;
The same replacement is done for patron's attributes too.
Test plan:
- Confirm that the new tests pass. That should be enough to confirm this change make sense.
Test plan (manual):
- Create a biblio with a title ending with a punctuation (like "with_punctuation_"), or any other fields of biblio/biblioitem
- Generate a notice which will display this field (CHECKIN for instance)
Use the historical syntax and the TT syntax, both should display the title without the punctuation character at the end
CHECKIN historical:
The following items have been checked in:
----
<<biblio.title>>
----
CHECKIN TT syntax:
The following items have been checked in:
----
[% biblio.title %]
----
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces two methods to be used by plugin authors:
->output
->output_html
They are basically wrappers for the helper methods from C4::Output
(output_html_with_http_headers and output_with_http_headers).
Plugin authors can use them, or keep the current flexibility of handling
the headers themselves in their code.
The KitchenSink plugin should be updated to highlight this.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Plugins.t
=> FAIL: The methods are not implemented
- Apply this patch
- Run:
k$ prove t/db_dependent/Plugins.t
=> SUCCESS: Tests pass, and they are meaningful
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should actually use Koha::Patron->checkouts here to avoid the
prefetch.
Test plan:
A patron with checkouts cannot get a discharge
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To move this subroutine out of the C4 namespace we face the same
problematic as bug 17553 (with GetOverduesForPatron).
We need to provide an equivalent method and so return all the related
value for a given checkout.
We can acchieve the easily using Koha::Object->unblessed_all_relateds,
but we need to keep in mind that it is a temporary move.
Indeed we will want to use our API to only access/retrive values we really need.
The whole trick could be removed when the current syntax for notices
will be deprecated and removed.
Note: this method returns the same number of elements than ->checkouts
They indeed returns the same things, but it sounds better to me to have a
different method to highlight (from the callers) where does it come
from (C4::Members::GetPendingIssues).
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In order to refactor our legacy code step-by-step, we will need to
provide code with the exact same behaviors and then improve it.
The same idea appears for the TT syntax for notices: we will want to
deprecate the existing syntax in order to support only one syntax.
Currently we fetch all the values from the related tables, without
knowing which are actually used.
I am suggestion to introduce a Koha::Object->unblessed_all_relateds
method which will return a hash containing all the fields from the
related tables (with the problems we know: collision in column names).
It is the existing behavior of GetOverduesForPatron and GetPendingIssues
for instance, they are used to send notices and so we have to provide
all the data needed.
See dependent bugs to understand the context and test this patch.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@deichman.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
prove -v t/db_dependent/Koha/Reports.t
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Maksim Sen <maksim.sen@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The first patch makes Hold.t fail on:
not ok 8 - Hold is suspended with a date, truncation takes place automatically
Failed test 'Hold is suspended with a date, truncation takes place automatically'
at Hold.t line 94.
got: '2018-03-22'
expected: '2018-03-22T00:00:00'
We could remove the midnight time there, but we also could replace ymd by
datetime. The cpan doc tells us:
Same as $dt->ymd('-') . 'T' . $dt->hms(':')
Test plan:
With this patch, run Hold.t again. It should pass now.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When a hold is suspended, 'suspend_until' is represented as a DateTime object, complete with locale. All told, this is about 800 lines of text.
Test Plan:
1) Enable the HoldsLog syspref
2) Add a hold on a record/item
3) Suspend the hold with a date to resume
4) Note the massive amount of date in the suspend_until field
5) Apply this patch
6) Suspend another hold with a date to resume
7) Note the log has an acutal date in the suspend_until field
Check the logs using module 'Holds' and Action 'Suspend'
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
TO test:
0 - Have a koha with ES enabled and some auth records
1 - backup your db
2 - sudo koha-mysql <<instance>>
3 - UPDATE auth_types SET authtypecode="PERSO_NAMB" WHERE
authtypecode="PERSO_NAME" (or adjust as necessary)
4 - Search for some authorities where a PERSO_NAME record will be
returned
5 - KA_BOOM!
6 - Apply patch
7 - Results are returned, they do not display a code, viewing record you
won't see code
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces two functions to the patrons endpoint:
- _to_api
- _to_model
This are in charge of field mappings in order to comply with the
guidelines.
Koha::REST::V1:Auth is adjusted to handle 'patron_id' as well. 'borrowernumber'
handling is kept until the existing endpoints get updated.
To test:
- Apply the patches
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/*.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
prove t/db_dependent/api/v1/patrons.t
--> without patch the list subtest fail
--> with patch test pass
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch removes previously added validation code from Koha::Patron
as we will rely on the DB structure and relationships to catch the same
problems. This is implemented on bug 19828.
This patch also adapts the API controller class to expect this behaviour
change from Koha::Patron. The expected exceptions are adjusted, and some
minor changes take place. The API tests are adjusted as well.
To test:
- Run:
$ kshell
k$ prove t/db_dependent/Koha/Patrons.t
k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests should still pass
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Works perfectly. Well done everyone!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch refactors the original work so it implements the controllers
and the spec using Mojolicious::Plugin::OpenAPI, and OpenAPI for the specification.
It removes the ability for patrons without permissions to edit their own data or their
guarantee's. This will be moved to a patron modification requests endpoint for simplicity.
It makes use of bugs 19410 and 19686 and their dependencies to deal with parameters handling,
query building and pagination.
Tests are adapted.
To test:
- Apply this patches and the dependencies
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
- Sign off :-D
Sponsored-by: ByWater Solutions
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds support for add, edit and delete patrons via REST API.
GET /api/v1/patrons Get patron list from params
GET /api/v1/patrons/<borrowernumber> Get single patron
POST /api/v1/patrons Create a new patron
PUT /api/v1/patrons/<borrowernumber> Update data about patron
DEL /api/v1/patrons/<borrowernumber> Delete a patron
Revised Test plan:
1) Apply this patch
2) Run tests perl t/db_dependent/api/v1/patrons.t
3) Add a user with proper rights to use the REST API
4) play with your favourite REST client (curl/httpie, etc.):
Authenticate with the user created above and get a CGISESSION id.
Use the CGISESSION to add, edit and delete patrons via the API.
5) Use PUT /patrons/<borrowernumber> for a patron without borrowers
flag. This should go into pending patron modification status and
needs to be accepted by a librarian.
Please note there is no validation of body input in PUT/POST other
than branchcode,category,userid,cardnumber.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We likely shoudln't pass through an uncoverted sort order for now, but
it does allow us to look ahead to implementing the orders directly so
seems a good option to have.
Either this patch should be used, or the commented out tests should be
removed
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Currently sort order is extracted from sort condition by splitting the
field, instead use regular expression to extract the last part preceded
by underscore.
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Enable Zebra
2 - Perform an auth search
3 - note results
4 - Enable ES
5 - Repeat search, note (likely) diff results
6 - Open a record in cataloging and use the button to launch auth search
7 - Perform same search as above, note results match for eiher engine
selected
8 - NOTE: Disbale sorting for ES search - this will be dealt with in
another report
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Edit a framework and add the 'upload/pl' plugin to the 856 field
2 - Attach a pdf to one record
3 - Attach a different type of file to another
(To attach, edit a record in the framework edited above and find the
helper by the 856 field)
4 - Click the links on the record
5 - Verify the pdf opens inline
6 - Verify the other file does not
7 - prove t/db_dependent/Upload.t
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The symbol of currencies are not displayed (or not correctly) so far.
This patch set adds the ability to configure the display of the symbol
(with or without a whitespace between the symbol and the price).
Test plan:
1. Execute the update DB entry, go to the currency admin page and tick the
new "Space separation between symbol and value" checkbox
2. Add a fine to a patron and use their credentials to login at the OPAC
3. You should see the "$ 42 due fines and charges" info in the
dashboard
4. Untick the new checkbox to remove the space and reload the OPAC main
page.
=> The space should not be longer displayed ($42)
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The template plugin did not pass the with_symbol flag to the module and
so was not taken into account
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Run through the password reset process, and your server
should send the message immediate, not waiting for the
cronjob.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
If you have an item-level hold, changing an attribute (like priority,
pickup location or suspended until date) makes the hold a biblio-level
hold, because ModReserve is not passed the current itemnumber.
This patch uses the hold's itemnumber and passes it to the ModReserve
call.
To test:
- Run the regression tests from the previous patch:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail
- Apply this patch
- Run the tests:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We do not need to format the date, DBIC handles DateTime correctly.
It fixes t/db_dependent/Letters.t with new SQL modes
Test plan:
prove t/db_dependent/Letters.t
prove t/db_dependent/Koha/Acquisition/Order.t
Must return green
Signed-off-by: Roch D'Amour <roch.damour@inlibro.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: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
As said previously, GetMemberAccountBalance returns ( the balance, the
non issues charges, the other charges)
The other charges are the balance - the non issues charges.
In this patch we remove the subroutine from C4::Members and use the
new Koha::Account->non_issues_charges subroutine instead
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The GetMemberAccountRecords may be a perf killer, it retrieves all the
account lines of a patron and then the related item and biblio
information.
Most of the time we only want to know how much the patron owns to the
library (sum of amountoutstanding). We already have this information in
Koha::Patron->account->balance.
This patch replaces the occurrences of this subroutine by fetching only
the information we need, either the balance, the detail, or both.
It removes the formatting done in the module, to use the TT plugin
'Price' instead.
There is a very weird and error-prone behavior/feature in
GetMemberAccountBalance (FIXME): as the accountlines.accounttype is a
varchar(5), the value of the authorised value used for the
ManInvInNoissuesCharge pref (category MANUAL_INV) is truncated to the 5
first characters. That could lead to unexpected behaviors.
On the way, this patchset also replace the GetMemberAccountBalance
subroutine, which returns the balance, the non issues charges and the
other charges. We only need to have the balance and the non issues
charges to calcul the third one.
Test plan:
Add several fees for a patron and play with HoldsInNoissuesCharge,
RentalsInNoissuesCharge and ManInvInNoissuesCharge.
The information (biblio and item info, as well as the account line) must
be correctly displayed on the different screens: 'Fines' module, fine
slips, circulation module
Note that this patchset could introduce regression on price formatting,
but will be easy to fix using the TT plugin.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixes two errors that slipped in the patchset.
To test:
- Create a dummy branch for testing:
$ cd kohaclone
$ git fetch
$ git checkout v17.11.00 -b dummy
- Reset your working DB
$ reset_all (y)
- Set your branch to current master
$ git reset --hard origin/master
- Update the DB
$ updatedatabase
- Update the schema files
$ kshell
k$ misc/devel/update_dbix_class_files.pl \
--db_name koha_kohadev \
--db_user koha_kohadev \
--db_passwd password
k$ exit
$ git diff
=> FAIL: There are discrepancies on upgrades
- Reset to v17.11.00 revision and DB:
$ git reset --hard v17.11.00
$ reset_all (y)
- Set your branch to current master
$ git reset --hard origin/master
- Apply this patch
- Update the DB
$ updatedatabase
- Update the schema files
$ kshell
k$ misc/devel/update_dbix_class_files.pl \
--db_name koha_kohadev \
--db_user koha_kohadev \
--db_passwd password
k$ exit
$ git diff
=> SUCCESS: No discrepancies!
- Reset to HEAD to get rid of the schema changes
$ git reset --hard HEAD
- Regenerate the schema files on top of this patch
$ dbic ; cd /home/vagrant/kohaclone
$ git diff
=> SUCCESS: No discrepancies!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should call Koha::Patron->is_expired in CanBookBeIssued instead of
doing the same calculation.
Tests have been adapted to pass with new SQL modes.
We should not need to update the values in DB, we already have
Bug 14717: Prevent 0000-00-00 dates in patron data (3.21.00.023)
Test plan:
prove t/db_dependent/Circulation/dateexpiry.t
prove t/db_dependent/Koha/Patrons.t
must return green
Signed-off-by: Roch D'Amour <roch.damour@inlibro.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 a new "Not for loan" status filter to the lost items
report.
Test plan:
0/ Apply all patches from this patch set
1/ Use the new "Not for loan" filter to search lost items
2/ The table result must be consistent and a new "Not for loan" column
should be there
3/ Confirm that you are able to hide/show this column with the column
settings tool.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Sponsored-by: Hotchkiss School
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Sponsored-by: Hotchkiss School
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
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 implements the changes required by the cities endpoint RFC
[1].
It uses the objects.search helper, and relies on bug 19686.
To test:
- Apply the patches
- Compare the spec with the RFC (api/v1/swagger/definitions/city.json)
=> SUCCESS: It makes sense
- Run:
$ kshell
k$ prove t/db_dependent/api/v1/cities.t
=> Tests pass!
- Sign off :-D
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In our installation procedure we ask the administrator to edit the
MySQL|MariaDB configuration file to specify the SQL modes we support
(see bug 17258 comment 6 and 7 for more information).
We are on the way to catch and fix all these issues and support these
stricter modes (as they highlight problem in our codebase/DB structure)
but in the meanwhile it may be good to remove this step and revert the
changes when we are ready.
TODO:
- Remove that for dev installations (to let developers catch these bugs)
- Edit the wiki page to remove this step
Test plan:
0. Do not apply this patch
1. Edit your MySQL|MariaDB config and add:
sql-mode = "ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION"
to the [mysqld] section (or edit it)
2. Restart your DBMS
3. Try to make the app explode (watch the logs)
(tips: you should get "'koha_kohadev.me.id' isn't in GROUP BY" when
editing an order)
4. Apply the patch, restart_all, restart your DBMS
5. Try to recreate the failure
=> You should no longer see the error in the logs
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>
This patch adds (yet) another param to objects.search: a reference
to a to_api function to be applied when processing the search results
for output.
To test:
- Apply this patch
- Run:
$ kshell
k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Test count raised, and tests pass!
- Sign off :-D
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 introduces a new parameter to the objects.search Mojo helper.
The idea behind this, is that if there's any attribute name mapping
between the DB objects and the API exposed ones, we should be able to
pass it to objects.search so the filtering query params are mapped
correctly for building the DBIC query, like this example:
my $patrons_set = Koha::Patrons->new;
my @patrons = $c->objects->search( $patrons_set,
\&to_model
)->as_list;
# and probably
@patrons = map {to_api($_)} @patrons;
The to_model function needs to avoid autovivification, to prevent
messing with the query parameters (undef params).
To test:
- Apply this patches
- Run:
$ kshell
k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t
=> SUCCESS: Tests pass! Params get mapped!
- Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Bourgault <david.bourgault@inlibro.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
When a test was introduced to not add duplicate userids
it overlooked that if you were matching on userid you
do not want the update to be rejected.
Add a test to overcome this logical flaw if updating
by matching userid
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch will have to be squashed with previous ones, but for
readability I prefer to let it alone for now.
It reintroduces 2 missing changes:
commit fd12390365
Bug 15010: Import patron tool creates 'duplicate' restrictions (
debarments )
commit 011d8e9855
Bug 12543 - Add userid as matchpoint for "Import patrons" tool
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Tests will have to be written for this change
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test Plan:
1) Apply this patch
2) Test importing patrons from command line,
options are availble with --help.
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested with minimal csv
(cardnumber,surname,firstname,categorycode,branchcode,password,userid)
Overwrite does not change category or branch.
Patrons are loaded, userid & password works
Updated license to GPLv3
No other koha-qa errors.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Tidy import_borrowers.pl
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Move importing code to a subroutine
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Update command line script to use patron import subroutine
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 [QA Followup]
* Fix copyright on import_borrowers.pl
* Changes -c --csv to -f --file
* Adds -c --confirm option
* Renames misc/import_borrowers.pl to misc/import_patrons.pl
* Restore userid matchpoint option
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Fix merge to master. Backport 3 updates from latest import_borrowers.pl
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Started regression tests. Fix missing C4::Members::Attributes package
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - More refactoring and regression tests in Koha::Patrons::Import
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Creating objects in misc/import_patrons.pl and tools/import_borrowers.pl
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Refactoring Koha::Patrons::Import includes bug fixed for critical date types and header column parsing
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598 - Rebase + backport of 16426 plus fixing 16426
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 12598: catch warnings raised by import_patrons in tests
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Not trivial, more attention needed on this one!
Fix for:
'koha_kohadev.me.cardnumber' isn't in GROUP BY
Looks like the query is complexified, perf tests needed, see bug 19599.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fix at ->store level, this issue appears in lot of places.
Fix for:
Field 'datecreated' doesn't have a default value
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Add POD for new sub in Koha/IssuingRules.pm.
Adding use Koha::IssuingRules to opac-detail and opac-MARCdetail.
Adding use Koha::Items to opac-detail and opac-MARCdetail.
Correct typo $items => $item in opac-MARCdetail.pl.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy
Test plan:
Make sure the onshelfholds policy is correct when placing a hold
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
- Checkout an item
- Place hold on this item,
- Return the item
- Make sure the hold is waiting (found W) and AllowOnShelfHolds is
not to 'Allow'
- Check that the button "Place hold" appears in opac detail page of
the biblio
- do the samewith items/reserves in transit
Changes on C4::Reserves::IsAvailableForItemLevelRequest
Make sure this tests pass:
- t/db_dependent/Reserves.t
- t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
Rebased - 2017-12-12 - Alex Arnaud
Bug 4319 - [QA fix] Create Koha::Biblio->hasItemswaitingOrInTransit
Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Perl::Critic found these violations in "Koha/Patron.pm":
"return" statement followed by "sort" at line 798, column 5. Behavior
is undefined if called in scalar context. (Severity: 5)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I do not exactly why but there is a conflict in the name of the method
prove t/db_dependent/api/v1/patrons.t failed with
[Mon Feb 12 17:13:16 2018] [error] Can't use string ("TO_JSON") as a
HASH ref while "strict refs" in use at
/home/vagrant/kohaclone/C4/Auth.pm line 2053.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Instead of basing the group searches on the group name, which is an
inherently touchy system, we should use the same checkbox style that
Jonathan introduced for the patron limits by group feature.
Test Plan:
1) Check to ensure existing group searches still show as they used to
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In order to control this feature correctly it needs to be enabled for
group of libraries and not for all groups defined in the system.
Groups will be used for different usages and so the feature must not be
enabled by default for all of them.
Test plan:
Retest bug 18403 with the feature turned on/off for a given tree
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
With one root group and two child groups and it works fine.
But if I create any groups under *those* groups, not only does it not work,
I get the following error:
Template process failed: undef error - SQL::Abstract::puke(): [SQL::Abstract::__ANON__]
Fatal: SQL::Abstract before v1.75 used to generate incorrect SQL when the -IN operator was
given an undef-containing list: !!!AUDIT YOUR CODE AND DATA!!! (the
upcoming Data::Query-based version of SQL::Abstract will
emit the logically correct SQL instead of raising this
exception) at /home/vagrant/kohaclone/Koha/Objects.pm line 269
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Marcel de Rooy
Janusz Kaczmarek
Jonathan Druart
Mark Tompsett
Kyle M Hall
Kyle M Hall
Nick Clemens
Tomas Cohen Arazi
Josef Moravec
Benjamin Rokseth
David Gustafsson
Tomas Cohen Arazi
Colin Campbell
Alex Arnaud