Coming from:
Bug 23463: Use new method Koha::Object->set_or_blank
We have DB fields that are not mapped with MARC fields, for instance paidfor. They are not handled correctly.
In ModItemFromMarc, we get a MARC record in parameter and update the item in DB. But we are loosing the fields that are not in the MARC record
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes
almost all perlcrictic violations according to this .perlcriticrc
The remaining violations are silenced out by appending a '## no critic'
to the offending lines. They can still be seen by using the --force
option of perlcritic
This patch also modify t/00-testcritic.t to check all Perl files using
the new .perlcriticrc.
I'm not sure if this test script is still useful as it is now equivalent
to `perlcritic --quiet .` and it looks like it is much slower
(approximatively 5 times slower on my machine)
Test plan:
1. Run `perlcritic --quiet .` from the root directory. It should output
nothing
2. Run `perlcritic --quiet --force .`. It should output 7 errors (6
StringyEval, 1 BarewordFileHandles)
3. Run `TEST_QA=1 prove t/00-testcritic.t`
4. Read the patch. Check that all changes make sense and do not
introduce undesired behaviour
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This reverts commit 80f1374f26.
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 is the same fix as on bug 14662, which fixed the behaviour in
cataloguing, but for the item form in acquisitions.
The code assumes that if a subfield is marked as mandatory, there
should be no empty entry in the pull downs.
This assumption is not correct, as it leads to the first entry of the
pull down being preselected if there is no default set. As the field
can never be 'unset', there will never be a 'required' warning.
Furthermore, it might be counterproductive to use mandatory fields,
as it might be easily forgotten to change the preselected value and
those mistakes will be hard to find.
Correct behaviour would be to preselect the empty value when there is
no default. This means on saving the item an error message is triggered
and the cataloger is forced to set the value.
To test:
- This is best tested with an ACQ framework, but default can be used
when no ACQ framework was created.
- In your MARC bibliographic framework:
- In 952 make itemtype, classification source and some other pull downs
like location or collection mandatory and set them to visibel if needed
- Create a new basket with 'items created while ordering'
- Add a new order, an existing record with 942$c set will work best
- Add items for your order line
- Verify that the first value of each pull down is preselected,
there is no way to trigger the 'required' error
- Apply patch
- Add a new order line
- Verify that classification source is preselected according to the
DefaultClassificationSource system preference (try unsetting it later)
- Verify all mandatory fields can be set to empty
- Verify that you can't save before correctly setting them
- Change your frameworks and set a default for itemtype (Ex: BK) and
another mandatory and non-mandatory field of your choice
- Add a new order line and item and verify the defaults are selected
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: 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>
Issue description:
- call to ModZebra was unconditional inside 'store' method for Koha::Item,
so it was after each item added, or deleted.
- ModZebra called with param biblionumber, so it is the same parameter
across calls for each items with same biblionumber, especially when we
adding/removing in a batch.
- with ElasticSearch enabled this makes even more significant load
and it is also progressively grows when more items already in DB
Solution:
- to add extra parameter 'skip_modzebra_update' and propagate it down to
'store' method call to prevent call of ModZebra,
- but to call ModZebra once after the whole batch loop in the upper layer
Test plan / how to replicate:
- make sure that you have in the admin settings "SearchEngine" set to
"Elasticsearch" and your ES is configured and working
( /cgi-bin/koha/admin/preferences.pl?op=search&searchfield=SearchEngine )
- select one of biblioitems without items
( /cgi-bin/koha/cataloguing/additem.pl?biblionumber=XXX )
- press button "add multiple copies of this item",
- enter 200 items, start measuring time and submit the page/form...
On my test machine when adding 200 items 3 times in a row (so 600 in
total, but to show that time grows with every next batch gradually):
WHEN ElasticSearch DISABLED (only Zebra queue):
- 9s, 12s, 13s
WHEN ElasticSearch ENABLED:
- 1.3m, 3.2m, 4.8m
WITH PATCH WHEN ElasticSearch ENABLED:
- 10s, 13s, 15s
Same slowness (because also same call to ModZebra) happens when you try
to delete all items ("op=delallitems"). And same fix.
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 depends on the framework parameter. Which should be added back to
the call in C4::Items.
Test plan:
[1] Mark in Default framework one subfield A repeatable and B not repeatable.
Go to item editor. (Work on a biblio in Default framework.)
Check saving and reopening these subfields with VAL1 | VAL2.
Subfield A should be cloned, B should be glued as entered.
[2] Mark in another Framework A not repeatable and B repeatable.
Change framework for this biblio.
Go to item editor again. Reopen item. Behavior subfields in reverse?
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch fixes an issue when editing items.
* The issue
Cannot blank a subfield when editing an item.
If you have an item with itemcallnumber=42, then edit it, blank it and
save. The itemcallnumber is still 42
* Why? (line numbers from https://gitlab.com/joubu/Koha/-/tree/bug_23463)
additem (and other item's edition forms) receives a list of tags,
subfields and values, generates a MARC::Record::XML then calls
ModItemFromMarc:
717 my $itemtosave=MARC::Record::new_from_xml($xml, 'UTF-8');
727 my $newitem = ModItemFromMarc($itemtosave, $biblionumber,
$itemnumber);
And ModItemFromMarc:
282 my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
283 $item->{cn_source} = delete $item->{'items.cn_source'}; #
Because of C4::Biblio::_disambiguate
284 $item_object->set($item);
ModItemFromMarc never knows that the field has been blank.
Prior to bug 23463 we had a map of default values, and ModItemFromMarc
was doing:
426 my $item = TransformMarcToKoha( $localitemmarc,
$frameworkcode, 'items' );
427 my $default_values = _build_default_values_for_mod_marc();
428 foreach my $item_field ( keys %$default_values ) {
429 $item->{$item_field} = $default_values->{$item_field}
430 unless exists $item->{$item_field};
431 }
I do not want to reinsert that list of default values.
Here I wrote a generic method in Koha::Object to set the value passed in parameter,
or "blank" if not passed.
It's nulled if can be set to null in DB, or the default value is retrieved from
the schema info.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
There should not be any calls left
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We did not do anything useful here.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
That's done in Koha::Item->store, and Koha::Object->store for integers.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Same as _set_defaults_for_add
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
From C4::Items to Koha::Item
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Starting to replace the ModItem calls with Koha::Item->store
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We are done with AddItem, only need to log and index.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Remove the unique call to _set_derived_columns_for_add
Even if design to deal with other values it only dealts with cn_sort
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
_set_defaults_for_add is no longer needed as it is handled by dbic and koha::object->store
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
It is no longer used.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch adds handling to catch transfers that may have been triggered
on item return.
Test Plan:
1) Enable AutomaticItemReturn and accompanying system preference
2) Return an item to the wrong branch and note the transfer
3) Check the database for the new trigger value
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We certainly faced 3 similar bugs due to this syntax: bug 23006, bug
22941 and bug 17526.
To prevent other issues related to this syntax this patch suggests to
replace them all in one go.
Test plan:
Confirm that the 2 syntaxes are similar
Eyeball the patch and confirm that there is no typo!
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>
It was displaying hundreds of:
Use of uninitialized value in concatenation (.) or string at
/kohadevbox/koha/C4/Items.pm line 862.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Test plan:
Check the tab order of the item block on additem.pl (item editor),
Acquisition (neworderempty or addiso2709), serials-edit.pl.
You could also check services/itemrecorddisplay.pl and pass a
biblionumber; this script is used in additem.js for cataloguing and
acquisition.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
its output is displaying tons of
Use of uninitialized value $field in regexp compilation at /kohadevbox/koha/C4/Items.pm line 2106.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
In items search, we can filter by items issues count, data coming from items.issues.
Most of the time, for an item with no issues this column contains NULL.
This enhancement proposes to consider NULL as 0 to allow searching items with no issues.
Test plan:
1) Use SQL to count items with items.issues = NULL :
select count(*) from items where issues is null;
2) Go to items search
3) Perform search with filter "Checkout count" = 0
4) Check you get the same number of results as SQL query
Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Note that we change both cataloguing/additem.pl and C4/Items->PrepareItemrecordfordisplay
I can find no code that uses callnumber from the C4/Items sub, except for the itemrecorddisplay script
which is not called with an itemnumber from Koha and should be deprecated for REST or ILSDI or OAI (imho)
To test:
1 - Define itemcallnumber syspref as "082ab,092ab,9520,245a"
2 - Find a record with no items
3 - Ensure it has no 082 field, but an 092 field
4 - Go to add an item - itemcallnumber is empty
5 - Apply patch
6 - Go to add item, itemcallnumber should be the 092ab fields
7 - Delete the 092 field
8 - Go to add item, itemcallnumber should be the 245a
9 - Edit the callnumber to be "testing" and save item
10 - For should now show itemcallnumber="testing" as default
11 - Browse to http://localhost:8081/cgi-bin/koha/services/itemrecorddisplay.pl?itemnumber=## subbing the correct itemnumber
12 - Ensure the callnumber is defaulting to testing
13 - delete the item you created
14 - browse to URL above - callnumber should now be 245 again
15 - Add an 092 field to record and ensure it is now default callnumber
16 - Add an 082 field, it should now be default
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
With bug 24217 pushed, lot of warnings appears during the tests are run.
Most of them are "Use of uninitialized value in "
Test plan:
Take a look at the output of run 1049
https://jenkins.koha-community.org/job/Koha_Master_D9/1049/consoleFull
Most of the warnings from this run will be removed by this patch
At least 2 are not fixed:
Use of uninitialized value in numeric eq (==) at /kohadevbox/koha/C4/Reserves.pm line 791.
t/db_dependent/Items_DelItemCheck.t => see 23463
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
We absolutely need that for modules.
We are enabling strict by using Modern::Perl.
Note that other modules from C4 use strict and warnings, instead of
Modern::Perl
Test plan: git grep 2505 **/*.pm should not return any results
And let's see later what needs to be fixed.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
The system preference AllowItemsOnHoldCheckout only affects SIP based checkouts. I believe we should rename it AllowItemsOnHoldCheckoutSIP so it matches AllowItemsOnHoldCheckoutSCO. We should also update the description as well.
As far as I know AllowItemsOnHoldCheckout has never affected anything except SIP traffic. That does not preclude the addition of another AllowItemsOnHoldCheckout syspref that would affect the staff interface, but that would be an enhancement and is outside the scope of this bug report.
Test Plan:
1) Test AllowItemsOnHoldCheckout via SIP
2) Apply this patch
3) Test AllowItemsOnHoldCheckoutSIP via SIP
4) Note there is no difference!
5) Note the syspref description has been update to mention SIP
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
t/db_dependent/Items/GetItemsForInventory.t .. 1/9 DBD::mysql::st execute failed: Expression #1 of ORDER BY clause is not in SELECT list, references column 'koha_kohadev.items.cn_sort' which is not in SELECT list; this is incompatible with DISTINCT [for Statement "
SELECT DISTINCT(items.itemnumber), barcode, itemcallnumber, title, author, biblio.biblionumber, biblio.frameworkcode, datelastseen, homebranch, location, notforloan, damaged, itemlost, withdrawn, stocknumber
FROM items
LEFT JOIN biblio ON items.biblionumber = biblio.biblionumber
LEFT JOIN biblioitems on items.biblionumber = biblioitems.biblionumber
ORDER BY items.cn_sort, itemcallnumber, title"] at /kohadevbox/koha/C4/Items.pm line 838.
We simply follow what the error says, and add items.cn_sort to the SELECT list
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
To test:
1 - Find an item
2 - Check it in at anothe rbranch and initiate a transfer
3 - View the item details or check db - note that holdingbranch is set as the destination library
4 - Complete or cancel the transfer
5 - Apply patch
6 - Initiate a new transfer
7 - Note the holdingbranch is set to the initiating library
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
If the cn_source has a default value defined in the framework it will
not be used when ordering or receiving serials.
Test plan:
Set the pref DefaultClassificationSource to Library of congress
Define a default value=ddc (for Dewey) for the ACQ and default framework
Receive an order (AcqCreateItem=on ordering) and confirm that Dewey is
selected by default (the default value from the ACQ framework is used)
Receive a serial and confirm that Dewey is selected by default
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
In serials receive and acquisition order creation from iso2709, the item edition form shows inputs that are missing "input_marceditor" class.
They are also missing tabindex and id.
The side effect is that those inputs dont get the "noEnterSubmit" class so form is posted if a barcode scanner is used or if enter is used on keyboard.
Test plan :
1)
1.1) Verify you have an item subfield linked to items.itemnotes, ie 952$z
1.2) Verify system preference "AcqCreateItem" is on "placing an order"
2)
2.2) Go to cataloguing module and edit an item
2.3) Verify that $z has an id, tabindex and "input_marceditor" class.
2.4) Verify that typing enter in $z does not submit the form
3)
3.1) Go to a serial with items creation, click on receive
3.2) Change status to "Arrived"
3.3) Verify that $z has an id, tabindex and "input_marceditor" class.
3.4) Verify that typing enter in $z does not submit the form
4)
4.1) Go to an acquisition basket
4.2) Click "Add to basket" and "From a staged file"
4.3) Click "Add orders" on a staged file
4.4) Go in tab "Item information"
4.5) Verify that $z has an id, tabindex and "input_marceditor" class.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch removed references to setting and getting the items.paidfor
field. Where it was used for display, in moredetail.pl, we replace it
with a query on the accountlines.
Test plan:
1) Apply patch
2) Pay off a LOST item
3) Check for the associated display of 'Paidfor?:' on the itemdetails
page
4) Writeoff a LOST item
5) Check that a 'Paidfor?:' is not displayed on the itemdetails page.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch introduces after-action hooks for
- C4::Biblio::{Add|Mod|Del}Biblio
- C4::Items::{Add|Mod|Del}Biblio
After the action has taken place, a call to a helper method is done,
which loops through all plugins implementing the hooks, and calls the
plugin method. The related object is passed, along with an 'action'
string specifying the action that took place, and the object id (which
is specially important for the 'Del' case).
To test:
- Apply this patchset
- Run:
$ kshell
k$ prove t/db_dependent/Biblio_and_Items_plugin_hooks.t
=> SUCCESS: Tests pass!
- Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch changes pending_hold to has_pending_hold to signify that
we're returning a boolean and not a Koha::Hold object.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
1) Apply all patches except this one
2) Checkout out an item to a patron
3) Check the item in - note that this will fail
4) Apply this patch, and repeat steps 1-3, noting instead that the the
check is instead successful
5) Verify also that reserves act as expected
5) Sign off
Sponsored-by: Catalyst IT
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a new syspref "UpdateItemLocationOnCheckin" which
accepts pairs of shelving locations. On check-in the items location is
compared ot the location on the left and, if it matches, is updated to
the location on the left.
This preference replaces ReturnToShelvingCart and
InProcessingToShelvingCart preferences. The update statement should
insert values that replciate these functions. Note existing
functionality of all items in PROC location being returned to
permanent_location is preserved by default. Also, any items issued from
CART location will be returned to their permanent location on issue (if
it differs)
Special values for this pref are:
_ALL_ - used on left side only to affect all items
_BLANK_ - used on either side to match on/set to blank (actual blanks
will work, but this is an easier to read option)
_PERM_ - used on right side only to return items to permanent location
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Set the new system preference UpdateitemLocationOnCheckin
to the following (assuming sample data):
NEW: FIC
FIC: GEN
4) Create an item, set its location to NEW
5) Check in the item, note its location is now FIC
6) Check in the item again, note its location is now GEN
7) Check in the item again, note its location remains GEN
8) Test using _ALL_, _BLANK_ and _PERM_ for updates
9) Try entering various incorrect syntax in the pref and note you are warned
Sponsored by:
Arcadia Public Library (http://library.ci.arcadia.ca.us/)
Middletown Township Public Library (http://www.mtpl.org/)
Round Rock Public Library (https://www.roundrocktexas.gov/departments/library/)
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a 'pending hold' column to C4::Items:GetItemsInfo to
allow for displaying status of an item when it has been selected for a
hold by the holdsqueue and AllowItemsOnHoldCheckout is set to 'Don't
allow'
To test:
00 - Set AllowItemsOnHoldCheckout to 'Allow'
01 - Place a hold on an item
02 - Build the holdsqueue (kohadevbox example below)
sudo koha-shell kohadev
perl misc/cronjobs/holds/build_holds_queue.pl
03 - Search for the item on the OPAC
04 - Note item shows as 'Available' in results and details
05 - Toggle AllowItemsOnHOldCheckout to 'Don't allow'
06 - Repeat search, note there is no change
06 - Apply Patch
07 - Search for the item on the OPAC
08 - Note that item now shows as 'Pending hold'
09 - Toggle AllowItemsOnHoldCheckout to Allow
10 - Note item shows as available
11 - prove -v t/db_dependent/Items.t
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Forced by removal of C4::Items::GetItem from Bug 21206
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Automatic item modification by age cronjob is based on rules with conditions and substitutions.
When substitution value is equal to actual item value, the code should not call C4::Items::ModItem. It adds unnecessary action log and entry in zebraqueue.
With a rule than can impact all catalogue you can explode your database with action logs.
Test plan :
1) Run prove t/db_dependent/Items/AutomaticItemModificationByAge.t
2) Define a item modification by age with no condition
3) Run several times misc/cronjobs/automatic_item_modification_by_age.pl
4) Check it creates only one entry in action_logs and zebraqueue
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
substitutions field stores the item field with 'items.' prefix.
Looks like call of C4::Items::ModItem() works but it will not if ORM object is used in the futur.
So I prefer fix it now and it is needed for main patch.
Test plan :
Run t/db_dependent/Items/AutomaticItemModificationByAge.t
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The PrepareItemrecordDisplay method used to generate the item
display on receiving serials was using a hardcoded maxlength
of 255 on input fields. This patch updates it to use the
maxlength defined in the frameworks instead as the normal
item form does.
To test:
- Create or reuse an existing subscription
- Make note of the framework the record uses
- Add an item on receive
- Check the maxlength on the various fields is set to 255
(check source code or use developer tools)
- You can also verify that by adding a long note to
one of the note fields
- Apply patch
- Repeat tests, the maxlength should now be 9999
- In your framework settings, change the length of one
of the subfields to another value
- Verify that the item form reflects the change
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
0/ Do not apply this patch
1/ Place several holds on a given item
2/ Use the inventory with the "skip waiting holds" option to list this item.
It will be displayed several times
3/ Apply this patch and confirm that the item is now only displayed once
Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Note: This is here for information purpose, feel free to test it if you
wan to play with it.
TODO: C4::Reserves::_get_itype is not longer in use
No more GetItem must be returned by:
git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep
-v GetItemsForInventory|grep -v GetItemsInfo|grep -v
GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v
GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v
GetItemSearchField|grep -v GetItemTypesCategorized|grep -v
GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The numerical restricted value was overwritten in GetItemsInfo
with the description. So OpacHiddenItems would only work for
restricted using the description, not the numerical value
of the authorised value like for other similar status fields.
This changes GetItemsInfo to use a separate variable for
the description, as it's already done for notforloan and
changes the template files where the variable is used
accordingly.
To test
- Before applying patch
- Configure OpacHiddenItems with:
restricted: [1]
- Find a record with more than one item and mark one of the items
as "Restricted Access"
(assume standard sample data - check authorised value if status doesn't exist)
- Verify the item is still shown in OPAC.
- Apply patch
- Verify that the item is now hidden
- Verify that the status still displays nicely in the staff detail page
- Delete OpacHiddenItems
- Verify that the status is showing nicely in the OPAC detail page
Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Like Bug 20702 defined GetHostItemsInfo does nothing if EasyAnalyticalRecords pref is disabled, get_hostitemnumbers_of must be dependant on this pref too.
Test plan :
1) Build an analytical record with 773$0 and $9
2) Enable EasyAnalyticalRecords
4) Place an hold on the record
5) => You see the linked item
6) Disable EasyAnalyticalRecords
7) Place an hold on the record
8) => You dont see the linked item
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 18816 missed the fact that a branch transfer triggers ModItem and
thus generates an action log line. We should not be creating those
action logs, especially considering the transfers table itself already
documents the transfer.
Test Plan:
1) Enable CataloguingLog
2) Transfer an item to another branch
3) Note the new action log line created
4) Apply this patch
5) Create another item transfer
6) No new action log line should be created!
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
1 - Catalog an item under LCC with callnumber GT95
2 - cn_sort should calculate as GT0095
3 - Go to inventory tool
4 - Enter a range that should have your item e.g LC GT90 to GT100
5 - Item is not returned
6 - Apply patch
7 - Item should be returned
8 - Verify things otherwise work as expected
Signed-off-by: Anne-Claire Bernaudin <anne-claire.bernaudin@univ-rennes1.fr>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
https://bugs.koha-community.org/show_bug.cgi?id=19905
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes the code using Koha::Biblio::Metadata->marcflavour use
->schema instead for all interactions.
To test:
- Update the DB structure:
$ updatedatabase
- Update the schema files:
$ dbic
- Notice all the places in which biblio_metadata is used
$ cd kohaclone
$ git grep biblio_metadata
=> SUCCESS: They all use `schema` instead of marcflavour
- Notice all the places that use Koha::Biblio::Metadata:
$ git grep Koha::Biblio::Metadata
=> SUCCESS: They all use the schema attribute when they used to use
marcflavour
- Run all the modified tests and scripts
=> SUCCESS: We are all good
- Sign off :-D
Note: while this seems like a minor change, the places in which plain
SQL is used really require understanding the queries and how they are
used, because some query results might be passed to some other method
that in turn uses the marcflavour attribute. I of course took that into
account but errare humanum est :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
Review the patch and check it is now updated correctly
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Also fully qualifies some subroutine calls that fail for reasons unkown.
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bug 10306 changed behavior on cloning item subfields by no longer splitting
constructions like 'A | B' in item fields like ccode.
If it is really recommended to clone item subfields, I am not so sure
about. But this patch at least restores the possibility to do so while
we discuss if we should ;)
Test plan:
[1] Run Items.t
[2] Make an item subfield repeatable in framework. And test edit items.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected. Also fixes the display of collections on the items
table (on editing items).
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The original WHERE condition wasn't enclosed in parenthesis, and hence
was subject to binding precedence issues in some situations.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch enables items to be skipped for inventory stocktaking
if the item is currently a waiting hold. The intention is to skip
items that should be on the holds waiting shelf in the library.
Test Plan:
1) Apply this patch
2) Generate a list of barcodes ordered by callnumber
3) Select one of those items and place it on hold, then confirm the hold
4) Remove that barcode from the list of barcodes
5) Browse to the inventory tool, choose your barcodes file
6) Run the inventory tool with that barcode file.
7) Note the tool says that item should have been scanned
8) Click the browsers back button to return to the previous page
9) Check the checkbox for "Skip copies marked as waiting holds"
10) Run the tool again, not it does not flag that item as previously
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
C4::Items::GetLastAcquisitions has been added by
commit 7753bbad4f
Adding Some new functions
(?)
Apparently it has never been used, we should remove it to avoid unnecessary maintenance.
Test plan:
git grep GetLastAcquisitions
must not return any results
NOTE: POD Coverage test failure is expected on removal
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Edit: Fixing merge conflicts in
- t/db_dependent/Items.t
- t/db_dependent/Search.t
- C4/Search.pm
Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems
To test:
1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items
Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Two lines of code and move them earlier.
Add a check to make sure there are changes.
Followed the test plan, both the _BEFORE_ and after do as described.
Signed-off-by: Cori Lynn Arnold <carnold@dgiinc.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Bulkmarcimport produces undefined subroutine error for
C4::Items::TransformMarcToKoha and C4::Items::GetMarcStructure
probably caused by mutually recursive modules. Put exports in
BEGIN clause before all the other imports.
To test:
1) Run bulkmarcimport.pl before applying patch and verify
that undefined subroutine error occurs
2) Apply patch
3) Run bulkmarcimport.pl again and verify that no errors are
produced
Sponsored-by: Gothenburg University Library
Signed-off-by: George Veranis <gveranis@gmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
git grep GetBarcodeFromItemnumber
should not return anything
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
http://lists.koha-community.org/pipermail/koha-devel/2018-August/044757.html
Prior to this patch, GetRecords returned timestampX, cardX and borrowerX for
the last 3 patrons who checked out the items.
I have no idea if it is a desired effects but, as this code has been
there for a very long time (2005), I suspect it's not.
The "doc" (/ilsdi.pl?service=Describe&verb=GetRecords) does not say
anything about the checkouts info.
Test plan:
hit /ilsdi.pl?service=GetRecords&id=1
and confirm the info about items are displayed correctly
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
C4::Items::GetItemnumberFromBarcode calls can be replaced with
Koha::Items->find({ barcode => $barcode });
We should make sure the barcode existed in DB and so that ->find
returns an object. Note that most of the time we just wanted to know if
the barcode existed.
The changes are very simple, the only one that need attention is
the one in batchMod.pl. It is basically reusing what we did on
bug 21141.
Test plan:
Use the batch item modification/deletion tools to modify/delete items
from their barcode (using the textarea or a file)
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
git grep GetItemnumberFromBarcode
should not return anything
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
When the syspref BlockReturnOfLostItems is set to Block, the item is blocked from being returned, but is still considered found -- it's set to lost=0 and a refund is applied to the patron (if circ rules allow). The item can then be checked in a second time and returned as it is no longer lost.
Test Plan:
1) Set an item to lost
2) Set BlockReturnOfLostItems to Block
3) Check the lost item in
4) Checkin message should say item is lost and cannot be returned
5) Check lost status of item, it should remain unchanged
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Triggered by the finding on bug 20697.
The three calls of GetHostItemsInfo should be controlled by the pref. This
patch makes the sub return an empty list when the pref is disabled.
The patch simplifies the sub by merging the two identical foreach loops
depending on the field number in MARC21/UNIMARC.
Will add a unit test on a follow-up patch.
Test plan:
See next patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
$frameworkcode is not used in ModItem (no need to pass it in
ModItemFromMarc).
$dbh is not used in ModItem.
Test plan:
Run t/db_dependent/Items.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The system preference CataloguingLog is not recommended for use in
production. This is do to the fact that every checkin and checkout
generates one or more log entires. This seems to be not only bad
behavior, but unnecessary and outside the needs of CataloguingLog as we
have CirculationLog.
Test Plan:
1) Log into staff client
2) Home -> Koha administration -> Global system preferences -> Logs
3) Set only CataloguingLog to 'Log', everything else to "Don't log"
4) Click 'Save all Logging preferences'
5) In MySQL, use your instance DB, and then type 'delete from action_logs;'
6) Have a person checkout and checkin anything.
7) In MySQL, 'select * from action_logs;'
-- there will be data. This is the floodiness that will be removed.
8) Apply this patch
9) Repeat steps 5-7
-- there should be no data.
10) Edit any biblio or item.
11) In MySQL, 'select * from action_logs;'
-- there should be data reflecting the changes made.
12) run koha qa test tools
NOTE: Improved clarity of test plan -- Mark Tompsett
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>
This patchset adds a 'damaged_on' column to store the date an item is
marked damaged, analogous to withdrawn_on and itemlost_on
To test:
1 - Apply patch
2 - Mark an item damaged via moredetail.pl (Items tab on left in
details)
3 - Note the damaged on date apears below
4 - Unmark the item, the date is removed
5 - Go to the edit items screen (from top bar 'Edit->edit items')
6 - Mark item damaged - check db or moredetails.pl to see damaged_on
date
7 - Unmark item damaged - confirm date is removed
8 - prove t/db_dependent/Items.t
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fix for:
DBD::mysql::st execute failed: Mixing of GROUP columns (MIN(),MAX(),COUNT(),...) with no GROUP columns is illegal if there is no GROUP BY clause [for Statement 'SELECT COUNT(*)
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>
We have found multiple open branch transfers for a given item. While I'm not sure how it is happening at this point, it seems that it would make sense to cancel any existing branch transfers when initiating a new one.
Test Plan:
1) prove t/db_dependent/Circulation/transfers.t
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
After feedback from the dev mailing list, it seems appropriate here to
propose making the Default framework authoritative for Koha to MARC
mappings. This implies checking only the Default framework in the
routines:
[1] GetMarcFromKohaField: The parameter frameworkcode is removed. A
follow-up report (19097) will update the calls not adjusted here.
This is safe since the parameter is silently ignored.
[2] GetMarcSubfieldStructureFromKohaField: Framework parameter is removed
and calls are adjusted. Includes acquisitions_stats.pl.
[3] TransformKohaToMarc: The parameter is removed; all calls are verified
or adjusted.
[4] TransformMarcToKoha: The parameter is no longer used and will be
removed in a follow-up report (19097). It always goes to Default now.
[5] TransformMarcToKohaOneField: The parameter is removed and all calls
are adjusted. Including: Breeding, XISBN and MetadataRecord modules.
[6] C4::Koha::IsKohaFieldLinked: This routine was called only once (in
C4::Items::_build_default_values_for_mod_marc. It can be replaced by
calling GetMarcFromKohaField. If there is no kohafield linked, undef
is returned. (Corresponding unit test is removed here.)
[7] C4::Items::ModItemFromMarc: The helper routine
_build_default_values_for_mod_marc does no longer have a framework
parameter. The cache key default_value_for_mod_marc- is no longer
combined with a frameworkcode. Three admin scripts are adjusted
accordingly; some tests will be corrected in the next patch.
Test plan:
See next patch. That patch adjusts all tests involved.
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>
In order to allow multiple Koha to MARC mappings (for one kohafield), we
need to adjust a few key routines in C4/Biblio.pm. This results in a few
changes in dependent modules too.
Note: Multiple mappings also include 'alternating' mappings. Such as the
case of MARC21 260 and 264: only one of both fields will be used. Sub
TransformMarcToKoha will handle that just fine; the opposite transformation
is harder, since we do no longer know which field was the source. In that
case TransformKohaToMarc will fill both fields. We only use that operation
in Koha for items (in Acquisition and Cataloging).
Sub GetMarcSubfieldStructure
This sub used a selectall_hashref, which is fine as long as we have only
one mapping for each kohafield. But as DBI states it: If a row has the same
key as an earlier row then it replaces the earlier row. In other words,
we lose the first mapping if we have two.
This patch uses selectall_arrayref with Slice and rearranges the output so
that the returned hash returns an arrayref of hashrefs for each kohafield.
In order to improve consistency, we add an order clause to the SQL
statement used too.
Sub GetMarcFromKohaField
This sub just returned one tag and subfield, but in case of multiple
mappings we should return them all now.
Note: Many calls still expect just one result and will work just fine:
my ($tag, $sub) = GetMarcFromKohaField(...)
A possible second mapping would be silently ignored. Often the sub is
called for biblionumber or itemnumber. I would not recommend the use of
multiple mappings for such fields btw.
In case the sub is called in scalar context, it will return only the first
tag (instead of the number of tags and subfields).
Sub GetMarcSubfieldStructureFromKohaField
This sub previously returned the hash for one kohafield.
In scalar context it will behave like before: it returns the first hashref
in the arrayref that comes from GetMarcSubfieldStructure.
In list context, it returns an array of all hashrefs (incl. multiple
mappings).
The sub is not used in C4::Ris. Removed the use statement.
Sub TransformKohaToMarc
This sub got a second parameter: frameworkcode.
Historically, Koha more or less assumes kohafields to be defined across all
frameworks (see Koha to MARC mappings). Therefore it falls back to Default
when it is not passed.
When going thru all mappings in building a MARC record, it also supports
multiple mappings. Note that Koha uses this routine in Acquisition and in
Cataloging for items. Normally the MARC record is leading however and the
Koha fields are derivatives for optimization and reporting.
The added third parameter allows for passing a new option no_split => 1.
We use this option in C4::Items::Item2Marc; if two item fields are mapped to
one kohafield but would have different values (which would be very unusual),
these values are glued together. When transforming to MARC again, we do not
want to duplicate the item subfields, but we keep the glued value in both
subfields. This operation only affects items, since we are not doing this
reverse operation for biblio and biblioitem fields.
Sub _get_inverted_marc_field_map
This sub is a helper routine of TransformMarcToKoha, the opposite
transformation. When saving a MARC record, all kohafields are extracted
including multiple mappings.
Suppose that you had both 260c and 264c in your record (which you won't),
than both values get saved initially into copyrightdate like A | B. The
additional code for copyrightdate will extract the first year from this
string.
A small fix in TransformMarcToKoha makes that it only saves a value in a
kohafield if it is defined and not empty. (Same for concatenation.)
Sub TransformMarcToKohaOneField
This sub now just calls TransformMarcToKoha and extracts the requested
field. Note that since we are caching the structure, this does not result
in additional database access and is therefore performance-wise
insignificant. We simplify code and maintenance.
Instead of modifying the passed hashref, it simply returns a value. A call
in C4::Breeding is adjusted accordingly. The routine getKohaField in
Koha::MetadataRecord is redirected to TransformMarcToKohaOneField.
NOTE: The fourth patch restructures/optimizes TransformMarcToKoha[OneField].
Sub get_koha_field_from_marc
This sub can be removed. A call is replaced by TransformMarcToKohaOneField
in C4::XISBN.
Note: The commented lines for sub ModZebrafiles are removed (directly under
TransformMarcToKohaOneField).
Test plan:
For unit tests and interface tests, please see follow-ups.
Run qa tools in order to verify that the modules still compile well.
Read the code changes and verify that they make sense.
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>
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>
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>
Change parameters to a hashref.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Looks good to me.
Two calls in migration_tools/22_to_30 still in old style.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fix POD error in Biblio.pm, as reported by qa tools:
Apparent command =cut not preceded by blank line
Resolve crash in t/db_dependent/Items/DelItem.t:
Can't call method "biblio" on an undefined value at C4/Items.pm line 669.
Add find test in tools/batchMod.pl. Increase readability of map statement.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To retrieve a biblionumber from an itemnumber, we can use:
Koha::Item->biblio->biblionumber
This is only what this patchset does.
Doing that we will be able to get rid of the
C4::Biblio::GetBiblionumberFromItemnumber subroutine.
Test plan:
- Acquisition module: cancel a receipt
- Export a record to CSV
- Modify items in a batch
Item's info should be correct
Other changes with be checked by QA team, by reading the code.
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>
At this point this subroutine is only used once, from reserve/request.pl.
Since we already have the items, it's easy to populate the different
hashes as the rest of the code is expecting it.
Test plan:
You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The JOIN done by this subroutine are not always useful (depending on
item-level_itypes). They also search with LIKE when it is not needed.
Since we have now Koha::Items, we can replace this subroutine with a
call to Koha::Items->search with the correct parameters.
A change in previous behaviours can happen: If a items.itemlost contains
a value that is not defined as a LOST authorised value, the item will
not be displayed. I think it's the expected behaviour, even if it should
not happen in correctly configured installations.
Test plan:
To test with item-level_itypes set to item and biblio:
List the lost items you have on your system, using the different
filters available.
The result table should contain the correct item's info.
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
