[1] Construction with a // b instead of a; unless( a ) b;
[2] Error checking on subfieldCode
[3] Add explanation how to fill preference
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
1. Run updatedatabase.pl
2. Set sysprefs MarcFieldForCreatorId, MarcFieldForCreatorName,
MarcFieldForModifierId, MarcFieldForModifierName
3. Create a new biblio
4. Verify that the fields are correctly filled
5. Logout and login as another user
6. Modify the same biblio
7. Verify that only the fields for last modifier have been modified
Works perfectly.
Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
[1] searchResults: second my $interface can be removed: unused
[2] call of getitemtypeimagelocation on L2119 needs interface key
[3] ISBDdetail: No need to find patron again (line 182 vs 84)
[4] opac-search: No need to find patron twice (657 and 631)
[5] tabs on line 2220 of C4/Search.pm (qa tools warn)
[6] Ugly hack to overcome "Undefined subroutine &C4::Items::ModZebra"
by loading C4::Items before C4::Biblio when running tests
Koha/BiblioUtils/Iterator.t and Labels/t_Label.t.
This is a more general problem that needs attention somewhere else.
It seems that Biblio.pm is one of the suspects.
[7] This patch set makes Search.t crash/fail with me. Note that without
these patches Search.t still passed! Why o why..
A little debugging pointed me to a missing MPL branch (aarg).
Adding the simple test on the result of Libraries->find in
C4::Biblio::GetAuthorisedValueDesc made the test continue.
[8] Resolve: Variable "$borcat" is not available at opac-detail.pl line 246
Lexical $borcat cannot be used in sub searchAgain in opac-detail.pl
under Plack. Must be defined with our (or passed as argument).
[9] Resolve crash on TWO serious typos in opac-basket on ONE line:
Koha::Patron->find({ borrowernumber -> $borrowernumber })
Yeah: find is in Koha::Patrons and we need => !!
No need to pass a hash to find method btw for a pk value.
[10] Serious bugfixing here. Add List::Util to opac-basket.
Can't locate object method "none" via package "1".
You can't test everything :)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
After this longer list I renamed Final to Additional in the patch title :)
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
- Added missing GetHiddenItems parameter change case
Without this prove t had a failure.
- Always use mocks, not set_preference
- Tweaks so t/db_dependent/00-strict.t passes
There was a typo botcat vs borcat and borrowernumber was never
defined. Grabbing from userenv, like other code does.
- Tweak t/db_dependent/Items.t to fully test changes
This will test all the if structures fully in GetHiddenItemnumbers.
prove t/db_dependent/Items.t
- Tweak borrower category code
$borrower->{categorycode} on a Koha::Patron is not the
same as $borrower->categorycode. Fixed error.
- Search was returning URLS for wrong interface
There was one search context place wrong. Changed it to $is_opac
as the logic for setting $is_opac was modified correctly.
- Corrected issues with category code.
When a user isn't logged in, $borrower is undef and causes error
when determining category code. Added conditional check.
- Properly trigger all changes in C4/Search.pm
- Fix QA Test tool failures
C4/Search.pm had some tabs.
- Add some commenting to make sense of logic
- Refactor EmbedItemsInMarcBiblio parameters to hashref
- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
prove t/db_dependent/Items.t
- Add missing test to trigger Koha/BiblioUtils/Iterator change
- Add borrower category overrides
These files generally add borcat parameter to GetMarcBiblio.
Others might include correction of filtering of items
(opac-basket), or a comment as to why no changes were done
(opac-search).
In the case of opac-search, correcting the first FIXME will
likely correct the OpacHiddenItems issues on tags. As such,
that is beyond this bugs scope.
Some code had loop optimizations and fixes made, like a
'next unless $record' when the biblio shouldn't even be in
the list.
- Modify opac-ISBDdetail and opac-MARCdetail
Both files had similar logic. They were rearranged and
optimized, so that both files would have practically identical
initial blocks of code.
Optimizations were possible, because GetMarcBiblio
returns a filtered record, so that there is no double call
(once in the opac-### file and once in GetMarcBiblio) to
GetHiddenItemnumbers.
- Fix hiding in opac-tags
opac/opac-tags.pl was not properly hiding.
There is currently one known bug associated with tags left.
If you have two biblios tagged by different people with the
same tag, the opac-search will show the one you tagged that
is supposed to be hidden, because tag searches work differently
than regular searches. This is beyond the scope of this bug.
See the FIXME's in opac/opac-search.pl
- Trigger the C4::ILSDI::Services changes
prove t/db_dependent/ILSDI_Services.t
- Added missing 'my'
- Test C4/Labels/Label.pm changes
- Improve C4::Record::marcrecord2csv test cases
- Corrected opac-details searchResult call
- Fix breaking issues constraint in ITerator test
- Fix ILSDI_Services test when clubs with branch exist
- Rebased again!
- Rebased t/db_dependent/Items.t conflict.
The test plan is in comment #112 last I checked.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To recreate:
- Go to a bibliographic detail page
- Delete it
- Go back
- Delete it again
=> Without this patch you will get a 500
Can't call method "holds" on an undefined value at
/home/vagrant/kohaclone/C4/Biblio.pm line 406.
=> With this patch applied it will silently redirect you to the search
form.
Note: We could/should improve the behavior and display a message, but
DelBiblio will need to be moved to Koha::Biblio->delete and other
callers adjusted
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This makes e.g. the advanced MARC editor, and anything that uses AddBiblio or
ModBiblio honor BiblioAddsAuthorities.
To test:
1. Make sure BiblioAddsAuthorities and AutoCreateAuthorities preferences are enabled.
2. Add a new record using advanced editor (enable EnableAdvancedCatalogingEditor to
use it), include a previously non-existing author.
3. Save the record and observe the author get an authority number.
4. Add another author, save the record and make sure it also gets an authority number.
Signed-off-by: Michal Denar <black23@gmail.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
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>
Add unsafe param to GetMarcSubfieldStructure and use this options
where it's safe to do so to increase performance
Sponsored-by: Gothenburg University Library
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds the possibility to define default indicators in
the MARC frameworks. It adds 2 columns in the marc_tag_structure table in
the database in order to accomplish this. All files that reference this
table have also been updated to reflect these added fields.
Test: Add or edit a MARC framework. In the Field list should be 2 extra
columns. It should be possible to add default indicators (1 character)
in these fields. Nothing else should have changed in the meantime.
The default indicator values are not yet visible in the cataloguing module.
The default values are also loaded in the cataloguing form.
Test: Define default values in some MARC framework. Go to cataloguing
and create a new record using this framework. Verify that the defined
defaults are visible when set. Verify the default is empty (as before)
if no default was set. Verify that if the default is changed, the
record is saved with the manually changed value. Verify that upon
changing such a new record, the manually set indicator value is used
and not the default one from the framework.
Don't forget to run database and database schema update
Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com>
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>
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>
This bug has been found during testing bug 19289.
In some conditions C4::Biblio::TransformHtmlToXml will generate a malformed XML structure. The last </datafield> can be duplicated.
For instance, if a call like:
my $xml = TransformHtmlToXml( \@tags, \@subfields, \@field_values );
with the last value of @field_values is empty, it will return:
<?xml version="1.0" encoding="UTF-8"?>
<collection
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.loc.gov/MARC21/slimhttp://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd"
xmlns="http://www.loc.gov/MARC21/slim">
<record>
<datafield2 tag="020" ind1=" " ind2=" ">
<subfield code="a">l</subfield>
</datafield>
<datafield1 tag="100" ind1=" " ind2=" ">
<subfield code="a">k</subfield>
</datafield>
<datafield1 tag="245" ind1=" " ind2=" ">
<subfield code="a">k</subfield>
</datafield>
<datafield1 tag="250" ind1=" " ind2=" ">
<subfield code="a">k</subfield>
</datafield>
<datafield1 tag="260" ind1=" " ind2=" ">
<subfield code="b">k</subfield>
<subfield code="c">k</subfield>
</datafield>
</datafield>
</record>
</collection>
Which will result later in the following error:
:23: parser error : Opening and ending tag mismatch: record line 6 and datafield
</datafield>
^
:24: parser error : Opening and ending tag mismatch: collection line 2 and record
</record>
^
:25: parser error : Extra content at the end of the document
</collection>
Test plan:
You can test it along with bug 19289 and confirm that it fixes the problem
raised on bug 19289 comment 30
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>
It is no longer used.
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>
This subroutine is only used once and can be replaced easily with a
Koha::Biblioitems->search call
Test plan:
Test this on top of bug 19941 and confirm that the correct item types
are displayed
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The subroutine GetBiblioItemByBiblioNumber considers that we have a 1-N
relation between biblio and biblioitems, which is wrong (it's 1-1).
So the calls can be replaced with Koha::biblio->biblioitem, it will ease
the read of the code.
Test plan:
1. Use the ILSDI service to display info of a bibliographic record,
biblioitems fields must be displayed
2. Search for items, biblioitems info must be displayed as well in the
result table
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>
Trivial fix. Problem raised by bug 10455.
Test plan:
[1] Create or edit biblio record.
[2] Save and check leader field lengths in MARC view.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Bug 14306 only handled field 555 in MARC21 as an URI. But a lot of other
5XX fields have a $u subfield for URIs; actually $u is not used in any
other way. This patch generalizes the change made for 555 and extends
it to all 5XX $u.
Test plan:
[1] Run t/Biblio.t
[2] Run t/db_dependent/Biblio.t
[3] Edit a MARC21 record. Add a URL into 505u, 520u, 555u.
[4] Check presentation on opac-detail (tab Title notes)
[5] Check presentation on catalogue/detail (tab Descriptions)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This form occurred in Dutch ISBD rules.
The question mark should follow the hyphen(s).
Test plan:
Run t/db_dependent/Biblio/TransformMarcToKoha.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
TransformMarcToKoha tests passed. Also this patch passed QA test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes it possible that uncertain year like 18.. or 197x are
converted to 1800 or 1970 in Koha field copyrightdate (MARC21) or
publicationyear (UNIMARC). (The corresponding MARC record will not be
changed obviously.)
This change will allow for better results when sorting search results or
list contents on copyrightdate. Currently, things like 18.. will sort
together with zero.
Note: The regex now allows four possible uncertain year markers: x or X,
question mark or dot.
Test plan:
[1] Run t/db_dependent/Biblio/TransformMarcToKoha.t
[2] Edit a biblio record. Save 18.. into 260c. Check biblio.copyrightdate.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Followed test plan, patch worked as described, it also passed QA test
tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
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>
This patch makes the following changes:
[1] Added POD for CountItemsIssued, GetBiblioItemData
[2] Moved TransformMarcToKohaOneField closer to TransformMarcToKoha (before
sub CountItemsIssued)
[3] Restructured TransformMarcToKoha by extracting individual kohafields via
TransformMarcToKohaOneField. The latter does no longer call the former.
This small optimization avoids traversing the whole MARC record again
and again.
[4] Moved adjusting copyrightdate/publicationyear to separate helper routine
_adjust_pubyear
[5] Removed obsolete sub _get_inverted_marc_field_map.
Test plan:
Run t/db_dependent/Biblio/TransformMarcToKoha.t
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>
Since the interface allows you to connect a kohafield to a MARC
controlfield, this routine should be able to handle that. Unfortunately
it did not.
Test plan:
Change will be tested in Biblio/TransformKohaToMarc.t in the next patch.
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>
[1] Replace corrosponding => corresponding
[2] Replace containts => contains
[3] Replace item_level-itypes => item-level_itypes
[4] Replace Managment => Management
[5] Replace should returns => should return
Test plan:
Note that this patch only deals with POD lines or test descriptions.
So there is nothing to test, just read the patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Patch amended by RM: The release notes should not be modified
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Use of uninitialized value in concatenation (.) or string at C4/Biblio.pm line 1456.
Note: In current master this is now line 1370 (Oct 9, 2017).
Test plan:
Enable COinSinOPACResults.
Select a record with leader pos6==a and pos7==a. This triggers genre to be
journalArticle and titletype to be a.
Without this patch, do an opac search that includes this record.
Check the log. You should see the warning.
Apply this patch, search again and check the log. The warning should not be
repeated again.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Test plan:
0. Apply patches
1. Update database
2. Go to administration -> libraries, try to update some library and
fill in some value into Marc Organization code field
3. Save this library and edit again - the code should be stored
correctly
4. Go to system preferences and fill in some value into MARCOrgCode
preference, note there is enhanced description mentioning the ability to
set organization code on library level
5. Set active library to the one with own org code stored
6. Go to cataloguing, create new empty record and click into field 003 -
there should be the code you filled for that library
7. Set active library to one withou marc org code
8. Go to cataloguing, create new empty record and click into field 003 -
there should be the code from system preference
9. Go to system preferences again and set AutoCreateAuthorities to
'generate' and BiblioAddsAuthorities to 'allow'
10. Go to cataloguing and create some biblio record, fill in any author
in to create its authority record, save the biblio
11. Go to authorities and find this created authority, go to details and
check the fields: 003, 040$a, 040$c, 670$a - there should be used right org code
12. prove t/db_dependent/AuthoritiesMarc.t t/db_dependent/Biblio.t t/db_dependent/Koha/Libraries.t
Signed-off-by: Hugo Agud <hagud@orex.es>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a new Koha::Hold->cancel method and replaces the calls
to C4::Reserves::CancelReserve with it.
Test plan:
- Add and cancel holds
- Change priority of holds
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
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>
Most of the time C4::Biblio::GetBiblioData is used to retrieve the title
and/or the author of a bibliographic record.
This patch replaces the easy occurrences of GetBiblioData, the ones
where the 2 joins are needed, but only data from biblio and biblioitems
table are.
Test plan:
It will be hard to test everything, I'd suggest a QAer to review this
patch and confirm that the difference occurrences of GetBiblioData have
been correctly replaced by calling Koha::Biblios->find or
$biblio->bibioitem
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
C4::Biblio::GetSubscriptionsId can be replaced using
Koha::Biblio->subscriptions
Test plan:
Create a new order for a bibliographic record
Create a new subscription on this biblio
From the basket (acquisition), confirm that you are not able to delete
the order with the biblio ("Can't cancel order and delete catalog record
1 subscription(s) left")
Receive the order
On the parcel page, confirm that you are not able to delete the order
with the biblio
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>
C4::Biblio::GetBiblio can be replaced with Koha Biblio->find
Test plan:
Import batch, view issue history, search for items, see the image of a
bibliographic record, modify and delete records in a batch
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>
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>
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>
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>
The C4::Koha::getitemtypeinfo subroutine did the almost same job as
GetItemTypes. On top of that it returned the imageurl value processed by
C4::Koha::getitemtypeimagelocation.
This value is only used from the 2 [opac-]shelves.pl scripts. Then it's
better not retrieve it only when we need it.
Test plan:
Play with the different scripts touched by this patch and focus on item
types. The same description as prior to this patch must be displayed.
Note that sometimes it is not the translated description which is
displayed, but that should be fixed on another bug report. Indeed we do
not expect this patch to change any behaviors.
Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Trims the URL in order prevent prefixing a space with http://
Normally you won't have a preceding space here, but I saw it happening
one day and it does not cost much to resolve it.
Bonus: Adding few simple tests in t/db_dependent/Biblio.t.
Test plan:
[1] Run t/db_dependent/Biblio.t
[2] Add a 856$u with preceding space (MARC21)
[3] Check opac-detail, Online access with OPACXSLTDetailsDisplay empty.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The C4::Reserve::GetReservesFromBiblionumber took 3 parameters, the
biblionumber, an optional itemnumber and a "all_dates" flag.
If set, the subroutine returned all the holds placed on a given bibliographic
record, even the ones placed in the future. Almost all of the calls had this
flag set, they will be replaced with a call to Koha::Biblio->holds.
But 5 did not have it:
- C4::Biblio::DelBiblio
-tools/batch_delete_records.pl
=> These 2 were wrong, we want to retrieve the holds to cancel them
before deleting the record. We need to get all the holds, even the ones
placed in the future /!\ CHANGE IN THE BEHAVIOR
- acqui/parcel.pl
=> 1 call per item were made to this subroutine. They have been replaced
with only 1 call to the new method Koha::Biblios->holds_placed_before_today
Then we filter on the itemnumbers.
I think this is wrong: we need the number of holds to know if the record
can be deleted, so even if future holds exist, the deletion should not
be possible.
- serials/routing-preview.pl
- C4::ILSDI::Services::GetRecords
- C4::SIP::ILS::Item->new
=> Seems ok, we just one to display holds placed before today
Test plan:
I would suggest to test this patch with patches from bug 17737 and bug 17738,
to place different kind of holds (biblio and item level, future and
past).
Then do a whole workflow to detect bug, view a record, delete record,
order, place a hold on an item which has been ordered, etc.
The hold's informations should always be the same without or without
these patches.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The 3 subroutines GetFieldMapping, SetFieldMapping and
DeleteFieldMapping from the C4::Biblio module were only used from the
field mappings admin page.
They can easily replaced with new packages Koha::FieldMappings based on
Koha::Object[s]
Test plan:
Add and delete field mappings (admin/fieldmapping.pl, Home ›
Administration › Keyword to MARC mapping).
Add an existing mapping > Nothing should be added
Note that this page has not been rewritten and you will not get any
feedbacks, but it's not the goal of this page to improve it.
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
There is a deletedbiblio_metadata table but it is not populated when a
biblio is deleted. Since we have a ON DELETE constraint on
biblio_metadata.biblionumber, the row is deleted when the biblio entry
is deleted => data lost!
Test plan:
- Create a biblio
- Delete it
=> Without this patch the deletedbiblio_metadata table is not populated
with the biblio_metadata row related to the biblio
=> With this patch applied you should see that the row has been moved.
Followed test plan, behaves 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: Brendan A Gallagher <brendan@bywatersolutions.coml>
This subroutine does not longer make any senses. The call to
get_infos_of can be replaced with $dbh->selectall_hashref.
The third argument of this subroutine was never used.
Test plan (for developer only):
Compare the 2 codes and confirm that they are equivalent
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Two discussions on koha-devel lead to the same conclusion:
biblioitems.marcxml should be moved out this table
- biblio and biblioitems
http://lists.koha-community.org/pipermail/koha-devel/2013-April/039239.html
- biblioitems.marcxml & biblioitems.marc / HUGE performance issue !
http://lists.koha-community.org/pipermail/koha-devel/2016-July/042821.html
There are several goals to do it:
- Performance
As Paul Poulain wrote, a simple query like
SELECT publicationyear, count(publicationyear) FROM biblioitems GROUP BY publicationyear;
takes more than 10min on a DB with more than 1M bibliographic records
but only 3sec (!) on the same DB without the biblioitems.marcxml field
Note that priori to this patch set, the biblioitems.marcxml was not
retrieved systematically, but was, at least, in
C4::Acquisition::GetOrdersByBiblionumber and C4::Acquisition::GetOrders
- Flexibility
Storing the marcxml in a specific table would allow use to store several
kind of metadata (USMARC, MARCXML, MIJ, etc.) and different formats (marcflavour)
- Clean code
It would be a first step toward Koha::MetadataRecord for bibliographic
records (not done in this patch set).
Test plan:
- Update the DBIC Schema
- Add / Edit / Delete / Import / Export bibliographic records
- Add items
- Reindex records using ES
- Confirm that the following scripts still work:
* misc/cronjobs/delete_records_via_leader.pl
* misc/migration_tools/build_oai_sets.pl
- Look at the reading history at the OPAC (opac-readingrecord.pl)
- At the OPAC, click on a tag, you must see the result
Note: Changes in Koha/OAI/Server/ListRecords.pm is planned on bug 15108.
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
There is only one call to C4::Biblio::ModBiblioframework, it's called
just before C4::Biblio::ModBiblio in cataloguing/addbiblio.pl
At first glance this call does not seems useful: all the subroutines
called from ModBiblio send the frameworkcode in parameter.
I'd go to remove it, but I'd like to get confirmation by others.
No test plan here, you need a good pair of eyes and deep into the
C4::Biblio code.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
C4::Biblio::GetHolds can be replaced with Koha::Biblio->holds->count
Test plan:
Create an order and place a hold on the biblio you have ordered.
On the basket view, you should not be able to Cancel the order and/or
delete the record
Receive the order, on the parcel page you should get the same behavior.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Any discussions about biblioitems.marc bring to the same conclusion:
This field is useless and should be removed.
We are storing MARC data into 2 different fields, that does not make
sense.
Test plan:
Add / update / export / import /delete records
should work as before
Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
What we currently have:
Koha/ElasticSearch.pm
Koha/ElasticSearch/Indexer.pm
Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
Koha/SearchEngine/Elasticsearch/Search.pm
What we want:
Koha/SearchEngine/Elasticsearch.pm
Koha/SearchEngine/Elasticsearch/Indexer.pm
Koha/SearchEngine/Elasticsearch/QueryBuilder.pm
Koha/SearchEngine/Elasticsearch/Search.pm
Test plan:
% git grep -i Koha::ElasticSearch
% git grep ElasticSearch|grep -v Catmandu::Store::ElasticSearch
should not return any result
Do a full reindex and search for records
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
REPLICATE ISSUE:
1. Map biblio.frameworkcode to 999$b
2. Map biblio.biblionumber to 999$c
3. Add a record with something in 999$b
4. 999$b is removed by C4::Biblio::AddBiblio()
After this patch, the field used by biblio.biblionumber or biblioitems.biblioitemnumber
is not removed and created anew, thus dropping all existing additions.
There is no point in dropping the field in any case, since we can just replace
the existing subfields in-place with no need to recreate the whole field.
UNIT TESTS INCLUDED
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
