This has been raised by failures on t/db_dependent/Circulation/issue.t
(thanks tests!)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 850243ee2e)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To recreate:
1 - Manually add a lost fine to a ptron and include a barcode
2 - Attempt to write off the fine
3 - Internal server error
4 - Checkout an item and mark lost to checkin and fine
5 - Attempt to write off line
6 - Internal server error
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 7b5c1fce58)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Bug 17898 provides a way of converting reports that use biblioitems.marcxml so that they will use biblio_metadata.metadata instead.
This only works with reports that do not refer to other columns in the biblioitems table. This is a known limitation. It means that we should be able to do a substitution of every occurrence of biblioitems with biblio_metadata, and every occurrence of marcxml with metadata.
Unfortunately, we're not doing a global replace, we're only replacing the first occurrence.
Test Plan:
1) Apply this patch
2) prove t/db_dependent/Reports/Guided.t
All tests successful.
Files=1, Tests=9, 10 wallclock secs ( 0.11 usr 0.01 sys + 2.85 cusr 0.25 csys = 3.22 CPU)
Result: PASS
Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit d6a1f2ab24)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Nothing important here, but this line should have been removed by bug
18966:
2177 # Update the fines
2178 $dbh->do(q|UPDATE accountlines SET issue_id = ? WHERE
issue_id = ?|, undef, $old_checkout->issue_id, $issue->issue_id);
The issue_id is now the same when moved from issues to old_issues. We do
not need to update the accountlines table.
No test plan here, you need to understand previous changes to validate
this patch.
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 0aec9e11d5)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Removed the SQL select condition 'AND closedate IS NOT NULL' because
this was not returning shippingcost values and it does not exist in the
SQL query to return the shipping cost in spent.pl
Also removed the retrieval of shipping cost and the associated addition
of item(s) cost and shipping cost in GetBudgetOrdered() in C4/Budgets.pm
to prevent the shipping costs being subtracted off the fund total twice
Test plan:
1. Go to Acquisition and create a currency, budget (make this value of
100), fund ( make this the value of 50), vendor (if
neccessary)
2. Create a basket and click 'Add to basket'
3. Add 2 items with the vendor price of 10
4. Click 'Receive shipment' and write in the shipment cost of 6
5. Click 'Finish receiving' and go back to Acquisitions
6. Notice the spent column value is 0.00 but if you click on the value then
the spent.pl page is displayed and shows that the shipment cost was 6.00
7. On the acquisition page also notice that the ordered column value is 26.00
8. Click on the name of the fund and notice the spent column value is
0.00 in the fund page table
9. Apply patch
10. Refresh acquisition page and notice that 6.00 is the value in the
Spent column and 20.00 is the value in the ordered column. Both of which
match the subtotal of the full-list tables displayed when you click on these
values
11. Also notice the spent value in the fund page table is 6.00
12 Observe the changes to GetBudgetSpent() and GetBudgetOrdered() C4/Budgets.pm and check they make sense
Sponsored-by: Catalyst IT
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit caef98c7f6)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Commit b4392018bc [Bug 12478: make things using SimpleSearch use the new version] changed sub FindDuplicateAuthority and replaced this call (from 3.22.x) in 16.05 (pushed April 2016):
my ($error, $results, $total_hits) = C4::Search::SimpleSearch( $query, 0, 1, [ "authorityserver" ] );
The new call does unfortunately not include the authorityserver:
my ($error, $results, $total_hits) = $searcher->simple_search_compat( $query, 0, 1 );
Simple_search_compat redirects to C4/Search/SimpleSearch and SimpleSearch assumes a biblioserver if no server is passed.
This effectively makes FindDuplicateAuthority useless since we will no longer find duplicates and we could see an error like this in the log:
16:51:42-04/10 zebrasrv(51) [request] Search biblios ERROR 114 1 1+0 RPN @attrset Bib-1 @and @attr 1=authtype PERSO_NAME @attr 1=Heading Moerenhout
The fix is obviously trivial.
Test plan:
[1] Go to Authorities.
[2] Add a new authority PERSO_NAME with 100a Moerenhout.
[3] Repeat step 2. Verify that you get the duplicate authority warning.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 56500e8e67)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Just replacing a wrong - by a correct + sign in the comment. The code
itself is fine.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 08357845c3)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test Plan:
1-go to C4/Accounts.pm
2-there should not be a trailing space at line 279.
Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 5400795a66)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Altough this patch deals with a mostly hypothetical case and this report
makes it practically impossible anymore to merge with records in the
Default framework (having no reporting tag), we can make the code of
sub merge still a bit more robust here.
If you would merge biblio records from one authtype to another and the new
framework would not have a reporting tag, before this patch the result would
be data loss. Merge would handle this request as a delete. This patch makes
merge handle it differently: instead of clearing the biblio records, it
keeps $a and $9 in order to make a future corrective merge possible.
Note: The additional condition on line 1468 for $tags_using_authtype
makes sure that we do not select all fields when the authtype should
unexpectedly be empty string (Default). This prevents crashing on
a "Control fields do not have subfields" error.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
(cherry picked from commit 434162c175)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
TEST PLAN
---------
1) Apply first patch
2) prove t/db_dependent/Acquisition/close_reopen_basket.t
-- FAILS
3) Apply this patch
4) prove t/db_dependent/Acquisition/close_reopen_basket.t
-- SUCCESS!
5) run koha qa test tools
Followed test plan, patch worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 632e2ad51d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Patron search fail on calling svc/members/search. This script
return a 500 error and the search stay on "Processing..."
Test plan:
- Enable ExtendedPatronAttributes system preference,
- make a standard search (search fields),
- check the search works and it doesn't stick on "Processing..."
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
I do not recreate the issue, but the change make sense and the issue has
been raised by several people
(cherry picked from commit 9c208a4431)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test:
1. find bibio without items which has something in field 773
(for us, it's article) but doesn't have 0 or 9 (host item entry)
2. click on hold in left menu
3. verify application error
4. apply patch and verify that it works
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(cherry picked from commit 5c873a408f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This patch makes C4::XISBN::get_xisbns() return an errors hashref
including information of the failing fetches from xisbn services.
It tackles the situation of XISBN, which in some cases returns 'overlimit'
errors.
The patch makes the relevant functions check if the response->{stat} string
is 'ok' and returns the string in $errors otherwise.
This only happens when in list context. This allows to fix the randomly failing
tests while keeping the current behaviour.
All this code should be rewritten. It does the job bug doesn't have problems handling
or reoprting. This is just a band aid.
To test:
- Make sure
k$ prove t/db_dependent/XISBN.t
=> SUCCESS :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit e908742418)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The variable is no longer used.
Removed a few empty lines on the way.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
0) Apply just the first patch - the one with test
1) Run t/db_dependent/Reserves.t - test for CancelExpiredReserves should
fail
2) Apply the second patch
3) t/db_dependent/Reserves.t should pass now
Followed test plan, patch worked as described. Passes QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Currently AddIssue tests if renewal, but logs an issue even if so. This
patch moves the logging into the conditional so a log entry is only
added if we aren't renewing (as renewals are logged separately)
To test:
1 - prove t/db_dependent/Circulation.t - one test should fail
2 - Enable both issue and renewal logs
3 - Checkout an item to a patron
4 - View the logs - the issue is captured
5 - Checkout the item to the patron again and confirm renewal
6 - Both an issue and a renewal are logged
7 - Apply patch
8 Repeat 1-6, tests should pass and only renewal should be logged
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit b66b86402c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Test plan:
0) Do not apply the patch
1) Place hold on item from another branch
2) Switch to that branch
3) Check them in at the other branch to set them into transport status (T)
4) Switch back to your homebranch
5) Check items in again, use the different confirm buttons and
compare: Only "confirm and print" will be set to waiting, "confirm"
remains in transport.
6) Apply the patch
7) Repeat 1-5 - now should work as expected - the hold is marked waiting
on "confirm" button too
8) Check the hold from the same branch, to make sure this doesn't add
regression
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 80fd5cd0db)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
As Jonathan pointed out, GetItem already called effective_itemtype.
So we can just use $item->{itype} here.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 0e744d49b5)
In the regular situation, you can get itemtype via biblio and then
biblioitem as well as via biblioitem (at least when item-level_itypes
is set to biblio).
But since Koha unfortunately defined two relations in item, one for
biblioitemnumber (the good one) and one for biblionumber (redundant),
TestBuilder (correctly) builds one biblioitem and two biblios.
If you item-level_itypes to biblio record, this will result in failing tests
when calling AddReturn (in this case Koha/Patrons.t).
It will crash on:
Can't call method "itemtype" on an undefined value at C4/Circulation.pm line 1826.
Cause: AddReturn goes via the biblionumber to biblio and than to
biblioitems, and it does not find a biblioitem. (Not a fault from TestBuilder
but a database design problem.)
This patch makes a small change in AddReturn to retrieve the itemtype via
biblioitem. It actually is a shorter road than items->biblio->biblioitems.
Note: I do not test the Biblioitems->find call, since we already checked
the GetItem call before and we have a foreign key constraint.
I did not call $item->effective_itemtype since we still use GetItem; this
could be done later.
Adjusted Circulation/Returns.t too: If we add an item with TestBuilder and
we called AddBiblio before, we should link biblioitemnumber as well.
Test plan:
[1] Do not apply this patch yet.
[2] Set item-level_itypes to biblio record.
[3] Run t/db_dependent/Koha/Patrons.t. (It should fail.)
[4] Apply this patch.
[5] Run t/db_dependent/Koha/Patrons.t again.
[6] Run t/db_dependent/Circulation/Returns.t
[7] Git grep on AddReturn and run a few other tests calling it.
Note: Bugs 19070/19071 address three tests that call AddReturn too.
[8] In the interface, check in a book.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Note: Bugs 19070 and 19071 are already pushed. The command in comment #4
has all the tests successful.
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 0fc3f19605)
On closing a basket, status is updated to ordered for orders not
completed. However the operation was resetting the status for
cancelled as well as new orders.
While display is correct from the basket view (it checks the
cancellation date). The status in the acquisitions tab from the
catalogue view reverts erroneously to ordered.
This patch adds cancelled to the statuses not updated on basket
close.
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>
(cherry picked from commit 2e9165001f)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1 - Set 'OverduesBlockCirc' to block
2 - Find or create a patron with overdues
3 - Perform a SIP patron lookup on that patron
misc/sip_cli_emulator.pl -a 127.0.0.1 -p 6001 -su term1 -sp term1 -l CPL
--patron {userid or cardnumber} --password {pass} -m patron_information
4 - Note the first character of response is a ' '
5 - Apply patch
6 - Restart memcached, apache, and plack
7 - Perform SIP patron lookup
8 - Note the first character of response is 'Y'
9 - prove t/db_dependent/SIP/Patron.t
10 - Test should return green
Signed-off-by: Chris Kirby <chris.kirby@ilsleypubliclibrary.org>
Works as advertised
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit ff4f085895)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
This subroutine is no longer in used and can be removed
Test plan:
git grep GetReservesToBranch
must not return any results
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 9cc7268362)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Code in C4::SMS takes the part of the SMS::Send-driver that comes after
SMS::Send and tries to turn it into part of a path to a YAML file
that can contain additional parameters to SMS::Send. The current
code works for e.g. SMS::Send::A::B, but if there is one or more
extra names, it fails to turn :: into /. So we have:
SMS::Send::A::B -> SMS/Send/A/B
SMS::Send::A::B::C -> SMS/Send/A/B::C
This patch makes sure all occurrences of :: are turned into /, by
adding a "g" modifier at the end of the regex.
Testing:
Testing this preperly would take a whole lot of setup for a very
small change. I would suggest that the following two oneliners
are enough to demonstrate that the change makes sense:
$ perl -e '$x = "a:🅱️:c"; $x =~ s|::|/|; print $x, "\n";'
$ perl -e '$x = "a:🅱️:c"; $x =~ s|::|/|g; print $x, "\n";'
So:
- Check that the output of these oneliners make sense
- Check that the patch changes the code in a similar way to the
change from the first oneliner to the second.
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit a4237785d9)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1 - Create an active budget
2 - Create an inactive budget
3 - Ensure they each have a fund with the same code
4 - Set MarcFieldsToOrder to get the budget_code from a marc field
5 - Stage a file using the duplicated code
6 - Add to a basket from the staged file
7 - Add the items
8 - Note funds are encumbered from the inactive budget
9 - Apply patch
10 - Repeat 5-8 with a new basket
11 - Note the active budget is now used
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 0e2c823e88)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The statement for head3 NB ('nota bene'?) looks like a hash key
in the list of possible return values for $needsconfirmation.
Moved it up and prefixed it with IMPORTANT.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 133e25c56a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The returns from C4::Circulation::CanBookBeIssued used
to be structured as a hashref of entries like
REASON => {
data => 'foo',
moredata => 'bar',
};
Some entries still are. But many are now
REASON => 1,
data => 'foo',
moredata => 'bar',
The sip Checkout routine still assumed the former, as it
reports any causes it was not aware of (to maintain support for
a changing api) The data fields could leak into the screen message
field of the response. e.g. the borrowernumber or surname of the
borrower who has a hold on an issued title. Some real messages were
getting obscured by this
This patch sanatizes the return from from CanBookBeIssued
by removing keys which are not all uppercase
It also fixes a case where the key's data element was used
for the screen message when we should use the key itself
Updated the documentation of CanBookBeIssued to flag up
the assumption re case and the fact that 3 elements rather
than two may be returned
The loop through the returned keys was a bit bogus
so we now explicitly jump out if noerror is unset
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested quite extensively. Test results put on Bugzilla.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 4d223bdc61)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
rebuild_zebra.pl fails in some conditions (perl version?)
I do not recreate but it has been reported that reindex fails with:
error retrieving biblio 94540 at /usr/share/koha/bin/migration_tools/rebuild_zebra.pl line 683, <DATA> line 751.
To fix it we can use fully qualified subroutine names for:
GetMarcFromKohaField
GetMarcBiblio
GetBiblionumberFromItemnumber
TransformKohaToMarc
GetFrameworkCode
Test plan:
Confirm the rebuild_zebra script still works correctly after this patch
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 2671eb2f93)
To test (You need a CAS server and CAS configured in Koha)
1/ Login using CAS in Koha
2/ Logout in Koha
3/ Notice you get redirected again and again
4/ Apply patch
5/ Login with CAS, then logout
6/ Notice logout works, but no longer infinitely redirected
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Patch has been in production use for several months
on several instances. Fixes a critical bug.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks like a typical workaround, but evidently works.
Not tested with CAS.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 13ca496206)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Resolves warnings like:
Use of uninitialized value in hash element at C4/XSLT.pm line 262.
Use of uninitialized value in numeric eq (==) at C4/XSLT.pm line 267.
Use of uninitialized value $status in concatenation (.) or string at C4/XSLT.pm line 300.
Note: Line numbers are not exactly the same anymore.
Test plan:
[1] Enable XSLT view in OPAC
[2] Without this patch, you will probably have some of these warnings when
doing some opac searches.
[3] Apply this patch. Check the logs again.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 5f989a81df)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
As requested by QA:
[1] Mock_config enable_plugins in the test.
[2] Fallback to MARC when format is empty. Remove die statement.
Added:
[3] Remove $marc. This variable got obsolete during development.
[4] Add test on $input_file and $plugin_class. Test $text before calling
Handler or processing $text. No need to split undef if somehow Handler
returned undef, etc. If the routine returns an empty arrayref,
stage-marc-import will do fine.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit a423fcff42)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Bug 12412 added the use of to_marc plugins allowing arbitrary file formats
in stage-marc-import (as long as the plugins can handle them). The feature
was not very visible in the code, and when bug 10407 added the marcxml
format, it made some changes that broke the use of to_marc.
This patch restores the functionality by:
[1] Adding a sub RecordsFromMarcPlugin to ImportBatch.pm, specifically
addressing the conversion from arbitrary formats to MARC::Record.
The original to_marc interface is used: pass it the file contents,
and it returns a string consisting of a number of MARC blobs separated
by \x1D.
Consequently, the call of to_marc is removed from routine
BatchStageMarcRecords where it did not belong. The to_marc_plugin
parameter is removed and two calls are adjusted accordingly.
[2] Instead of a separate combo with plugins, the format combo contains
MARC, MARCXML and optionally some plugin formats.
[3] The code in stage-marc-import.pl now clearly shows the three main
format types: MARC, MARCXML or plugin based.
Note: This patch restores more or less the situation after bug 12412, but
I would actually recommend to have the to_marc plugins return MARC::Record
objects instead of large text strings. In the second example I added a
to_marc plugin that actually converts MARC record objects to string format,
while RecordsFromMarcPlugin reconverts them to MARC::Records.
Test plan:
See second patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
(cherry picked from commit d24568b672)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 1262962a7d)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
From the syslog call on line 107/108. The field $item->{title} has not
been initialized. When catching the warns in a test script with Test::Warn,
I got:
Use of uninitialized value $args[1] in sprintf at /usr/lib/x86_64-linux-gnu/perl/5.20/Sys/Syslog.pm line 423.
This simply resolves the warning. We might have a closer look to this title
field on another report though.
Test plan:
Run Message.t from the next patch without this fix. It will fail since the
additional warning makes warnings_like return false.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 66a7565995)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
As per bug 18966 AddReturn returns false with a DataCorrupted message
when the issue_id at hand already occurs in the old_issues table.
This should be handled when returning an item via SIP too. SIP should
not pretend that the checkin was successful, since this item needs
special care.
The following six different situations are handled:
[1] An attempt to checkin an invalid barcode is handled as before. The
ok flag is false; the screen message shows 'Invalid Item'.
[2] We receive a DataCorrupted message: the alert type is set to the
unused 98 code to indicate this new error condition. The ok flag
is false; the screen message shows 'Checkin failed: data problem'.
[3] The item checked in was not checked out AND the option checked_in_ok
is active. The ok flag is set to true; no screen message.
[4] The item checked in was not checked out AND the option checked_in_ok
is not active. The ok flag is not changed (normally false); the screen
message shows 'Item not checked out'.
[5] (The regular checkin) The item was checked out and AddReturn returned
true. The ok flag is true; no screen message.
[6] ("Otherwise") The item was checked out, but AddReturn returned false.
The ok flag is false; the screen message shows 'Checkin failed'.
Note: Currently this case only refers to the Wrongbranch and withdrawn
messages from AddReturn (where doreturn==0).
Note: Situation 1 and 5 are unchanged. Behavior is slightly changed for
situation 3 and 4; the option is only used when the item was not checked
out. Situation 2 and situation 6 are changed. After bug 18966 SIP
returned OK in case of data corruption (while checkin failed). In the
remaining cases under [6] SIP also returned OK while checkin failed.
Test plan:
[1] Test all six cases listed above. In order to simulate data corruption
insert the issue_id at hand in old_issues before you check in.
In cases 3 and 4 you need to toggle the option in your SIPconfig.xml and
restart the SIP server.
Case 6 can be tested by checking out at branch A, adjusting the value
of pref AllowReturnToBranch and checking in at branch B.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 97ec4ae98c)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Replaces TRUNCATE by DELETE, since truncate implicitly commits. We don't
need to do that here. (Would complicate testing it too.)
Fixes typo disablig.
Add a simple test to HoldsQueue.t.
Test plan:
Run t/db_dependent/HoldsQueue.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit db64c94953)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
To test:
1 - Set 'OpacRenewalBranch' to various settings
2 - Renew an item for a ptron under each setting
3 - Confirm action_log entries reflect the correct branch for each
secnario
4 - prove t/db_dependent/Circulation/issue.t
Signed-off-by: David Kuhn <techservspec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit e9374dcf63)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Koha suffers of big bugs due to its history: When data are deleted, they are moved to another tables.
For instance issues and old_issues: when a checkin is done, it is moved to the old_issues table.
That leads to a main problem that is described on https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
However we tried first to fix the problem (for issues/old_issues) at code level on bug 18242.
The goal was to prevent data lost.
Data lost may happens in this case:
Check an item out (issue_id = 1)
Check an item in (issue_id = 1)
Restart MySQL (reset auto increment for issue_id to 1)
Check an item out (issue_id = 1)
Check an item in => BOOM, the issue_id is a PK in old_issues and the move fails.
Before bug 18242 the data were lost, we inserted the value into old_issues, which fails silently (because of RaiseError set to 0 in Koha::Database), then delete the row from issues.
That has been fixed using a transaction.
This patch introduced a regression we tried to fix on bug 18651 comment 0, the patron was charged even if the checkin was rejected.
A good way to fix that would have been to LOCK the tables:
1- Start a transaction
2- LOCK the table to make sure nobody will read id and avoid race conditions
3- Move the content from one table to the other, dealing with ids
4- UNLOCK the table
5- Commit the transaction
But there were problems using LOCK and DBIx::Class (See commit 905572910b - Do no LOCK/UNLOCK the table).
Finally the solution implemented is not acceptable for several reasons:
- batch checkins may fail
- issue_id will always stay out of sync (between issues and old_issues)
See 18651 comment 66.
Since the next stable releases are very soon, and we absolutely need to fix this problem, I am suggesting to:
1- Execute the move in a transaction to avoid data lost and reject the checkin if we face IDs dup
=> It will only reject 1 checkin (max is 1 * MySQL restart), no need to deal with race conditions,
2- Display a warning on the checkin page and link to a solution/explanation
3- Communicate as much as we can on the proper fix: Update auto increment values when the DBMS is restarted - https://wiki.koha-community.org/wiki/DBMS_auto_increment_fix
4- Display a warning on the about page for corrupted data (see bug 18931)
5- Write and make available a maintenance script to fix corrupted data (TODO LATER)
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
TEST PLAN
(reproducing bug)
1 - Set 'IndependantBranches' to enable
2 - Log in as Superlibrarian.
3 - Create 3 rules
- 1 for all librairies (1)
- 1 for the library of the superlibrarian (2)
- 1 for another library (3)
4 - Try to delete them and see them and see that (3) isn't the good one
(checking patch)
5 - Apply patch
6 - Try again 3-
7 - Connect as not superlibrarian
8 - Try again 3-
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 648e2a2b80)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Do not fill @return if retval == -1 for LDAP (see cfc484b17).
No need to call store after an DBIx update. Rearranged the if statement.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 313e1f2f27)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
A regression in commit cfc484b17 / bug #18314 breaks the local
authentication fallback for all external authentications like LDAP, CAS
and Shibboleth.
The regression itself is a logical error as "@return = (0)" is
considered to be "false" when checked with "unless" (line 1814). That's
wrong as "unless" tests the number of elements in a list.
This patch tries to simplify the logic by adding a $passwd_ok and
$check_internal_as_fallback flags to be more verbose and hopefully more
understandable.
The goal here is simply to restore back the same logic as before cfc484b17
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit d852b33266)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
There was a bug that meant a very large offset in the search params
will cause the search script to run forever (or long enough to crash
the machine)
To test
1/ Get ready with sudo top so you can kill the thread before it causes
your machine to OOM
2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999
3/ Notice the process runs for a long time
4/ Kill the process
5/ Apply the patch
6/ Hit the page again, notice the it loads (offset is set to zero)
7/ Do the same to search in the staff client
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit aabf001232)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Removing all the id from the columns on the inserts and removing the
parameter '' of the values for the id.
Test plan:
1) Go to tools -> calendar
2) Add a Holiday only on this day.
3) Add a Holiday repeated every same day of the week.
4) Add a Holiday repeated yearly on the same date.
5) Add a Holidays on a range.
6) Add a Holidays repeated yearly on a range.
7) You should have the five calendars displayed.
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit b94b16f679)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
With this patch a parameter 'allow_empty_passwords="1" can be added to a
login in the SIP configuration file to allow the behaviour as was normal
before the patch for bug 16610 was applied. Some sip clients rely on
this behaviour sending an empty password field when they wish to
validate to user but do not have the password.
If a password is supplied it will be validated
A test has been added to Message.t to confirm this behaviour
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit ebb887e5c9)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
The commands in the test plan are examples, and may need varying
depending on your installation. This was created as a result
of attempting to clean the installation process up. However,
I believe the redefine might exist normally too. I just didn't
check. This is tested on a Debian 8 box sudo apt-get update'd
fully.
TEST PLAN
---------
empty error log
$ echo > ~/koha-dev/var/log/koha-error_log
drop and recreate and empty db
> drop database koha_library;
> create database koha_library;
> quit
run the web installer, but DO NOT LOG IN!
*opening chrome to Staff Client URL*
check the error log
$ less ~/koha-dev/var/log/koha-error_log
...
[Fri Jun 09 13:08:52.793627 2017] [cgi:error] [pid 5802] [client 192.168.71.101:58169] AH01215: [Fri Jun 9 13:08:52 2017] CGI.pm: Subroutine multi_param redefined at /usr/share/perl5/CGI.pm line 419.
...
apply patch
empty error log
$ echo > ~/koha-dev/var/log/koha-error_log
refresh the installation login page
recheck the error log
$ less ~/koha-dev/var/log/koha-error_log
notice no reference to "Subroutine multi_param redefined"
run koha qa test tools
Notice that it is just a require CGI; and comment added.
Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit d222da0602)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Due to bad use of grep syntax if there is one or more Basket Users the result of grep is not equal to 0 and the borrower is allowed.
Test plan :
1- select system preference 'AcqViewBaskets' on 'user'
2- create 2 borrowers (A, B) with only permissions on acquisition :
group_manage
order_manage
order_receive
staff
3- login with A and create a basket
4- add a basquet manager other than B
5- relog with account B
6- you can see the basket
Apply the patch.
The basket is no longer visible.
1- relog with A
2- add basquet manager B
3- relog with B
5- you must see the basket
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>
(cherry picked from commit 0c09adbfc8)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
TEST PLAN
----------
git grep -i getsupplierby
-- only the code removed and the test tweaked
git bz apply 18782
sudo koha-shell -c bash kohadev
prove -v t/db_dependent/Serials.t
qa -v 2 c 1
exit
-- sign off
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(cherry picked from commit c69e02c441)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
GetFictiveIssueNumber:
Returns undef instead of 0 for irregular frequencies. Also added to POD.
Removed unused variable $wkno.
Adding a return makes the if(unit) unneeded.
Replaced (a+b)/b by 1+a/b.
_delta_units:
Added a comment about its parameters.
GetFictiveIssueNumber.t:
Adjusted the tests for irregular frequencies accordingly.
Test plan:
[1] Run t/db_dependent/Serials/GetFictiveIssueNumber.t
[2] Run t/db_dependent/Serials/GetNextDate.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(cherry picked from commit 4e9701c36a)
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Jonathan Druart
Kyle M Hall
Alex Buckley
Marcel de Rooy
Dominic Pichette
Mark Tompsett
Alex Arnaud
Dobrica Pavlinusic
Tomas Cohen Arazi
Josef Moravec
Nick Clemens
Colin Campbell
Magnus Enger
Chris Cormack
Baptiste Wojtkowski
Rodrigo Santellan
Christophe Croullebois
Julian Maurice