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>
The fails in the previous test showed that we need the first three
changes here. Some final polishing in points 4 to 6.
[1] Sub merge: Refine the condition for initializing $tags_new.
A postponed 'modify'-merge (A to B) makes that $authtypefrom is not
defined when running merge later. When crossing the type boundary, we
need a new field too.
[2] Sub merge: Add condition for an empty @record_to array.
This indicates also that a field should be removed, since we should
otherwise only add a $9 subfield.
[3] Sub merge: Adjust initializing @record_from.
This change is tested by verifying a cleared subfield in the test.
[4] DelAuthority: Adding a skipmerge parameter to allow the call from
authorities/merge.pl to skip an unneeded merge.
This also prevents that the 'delete-merge' would precede the
'modify-merge' under a hypothetical race condition.
[5] DelAuthority: There is actually no need to call GetAuthority.
The subfields of the old record are not relevant in this case.
[6] Added a few POD lines to merge.
[7] Removed a trailing space in a comment line in merge.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t.
The last subtest should no longer fail now.
[2] See test plan of next patch.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
At this point, we are replacing dontmerge functionality by the new
AuthorityMergeLimit logic. Instead of doing this check before calling
merge, we just call merge and check it there. In order to let the cron
job do the larger (postponed) merges, we add a parameter override_limit.
A subtest is added in Merge.t to test the 'postponed merge' feature. Since
merge now also calls get_usage_count, an additional mock is added. All
references to dontmerge are removed.
In merge two lines, initializing $dbh and $counteditbiblios, are moved.
The dontmerge test in DelAuthority and ModAuthority is removed. Since this
did not leave much in ModAuthority, I fixed the whitespace on the remaining
lines rightaway (yes, I know).
A minimal set of changes is applied to the cron script; it will get further
attention on a next patch.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Set AuthorityMergeLimit to 2. Modify an authority with two linked
biblios. Check that the merge was done immediately.
[3] Now modify an authority with more than 2 linked records.
Verify that the merge was postponed; a record must be inserted in
the need_merge_authorities table.
[4] Testing of the merge cron job is *postponed* to a next patch.
Note: I tested a modification, but the script just needs more
attention.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Since we can now call linked_biblionumbers, we can now remove all Zebra
related code from merge. We also add a parameter biblionumbers; we use it
in the test now, but it may be handy too later in the maintenance script
when we want to trigger a merge for specific biblionumber(s). See bug
report 18071.
All mocks for ZOOM, Context::Zconn, Search::new_record_for_zebra in the
merge test can now be replaced by one mock for linked_biblionumbers. Note
that we test the biblionumbers parameter in the last subtest without that
mock.
Remove unused vars $countunmodifiedbiblio, $counterrors from merge.
Renamed zebrarecords to linkedrecords in the Merge test.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Modify an authority record. Check the linked biblio records.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
We will need a few additional parameters for merge later on. This patch
puts the original parameters in a parameter hash.
For the same reason DelAuthority gets a parameter hash here.
Note: We remove the second parameter from the DelAuthority call in
authorities/authorities-home.pl here. It was not used and could have
presented problems in the future.
Test plan:
[1] Run t/db_dependent/AuthoritiesMarc.t.
[2] Run t/db_dependent/Authorities/Merge.t.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Resolves:
*** ERROR: Spurious =cut command at line 1376 in file C4/AuthoritiesMarc.pm
Test plan:
[1] Run podchecker on C4/AuthoritiesMarc.pm
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Script tools/batch_delete_records.pl only checks the return value of
DelAuthority. The return value depends on DBI rows, which is not always
reliable. It may return -1 when it 'does not know'.
Testing $@ in tools/batch_delete_records.pl should actually be enough.
The return value was discarded in C4/ImportBatch.pm. Removing an unused
variable.
Test plan:
[1] Pick an authority record with a few linked biblios.
Delete this authority record via tools/batch_delete_records.pl.
Check if the linked biblio records are cleaned up.
[2] Bonus: Make a typo in the SQL statement of DelAuthority. Check
if batch_delete_records shows you the error message.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In order to accomplish this, we need to add some additional checks in
the merge routine. The actual change to remove the field, is quite
small.
Furthermore, we need to add a merge call in DelAuthority and adjust
the merge cron job accordingly.
The change is well supported by additional tests, including a simulation
of postponed removal via cron, if dontmerge is enabled.
Note: Deleting an authority with linked biblios is tested on the next
patch.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Delete an authority without linked biblios from the Authorities
module. If the indexer is not fast enough, wait a bit and refresh to
verify that the authority is gone on authorities-home.pl.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Instead of using the MAX(authid)+1 logic, AddAuthority should just save
the record and get the new id. The authid column is an autoincrement.
This eliminates problems where a newly assigned authid also refers to a
previously deleted record. (And it will not cause problems when refining
the dontmerge functionality on report 9988.)
Note: ModAuthority also calls AddAuthority to update an existing record; in
that case we should not create a new record even if the record should not
be found any more (which should be exceptional).
This patch also simplifies handling of 001 in the authority record: in all
cases this field is updated now; no need to check its contents.
Test plan:
[1] Run t/db_dependent/AuthoritiesMarc.t
[2] Add a new authority record via the interface
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
We need to add $9 to the skip_subfields hash too. Formerly, it was
added to $exclude as well.
Thanks, Julian, for catching this one.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Kept the same number of lines.
You could verify with diff -w.
Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] As the last patch in this series, also test the interface:
Set AuthorityMergeMode to loose. Set dontmerge to Do.
Modify an authority record attached to multiple biblios.
Edit a subfield, clear a subfield and add a subfield.
Save. Wait a bit for the merge and Zebra update.
Verify that the changes are merged properly into biblio records.
[3] Repeat step 2 with AuthorityMergeMode to strict.
Remember that this affects the extra subfields in biblio records.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Remove some commented warnings
Remove the commented old code at the end of sub merge
Explicitly set merge mode in the first subtest
Move the return to loose mode from the second subtest to the third
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Since strict mode does not allow additional subfields that would make
identical fields linked to the same authority different, there is no
need to keep them while merging.
We achieve this goal by simply:
[1] Count the number of same fields linked to mergefrom in strict mode to
eliminate duplicates.
[2] Replaces the if-statement on auth_number by a next. (Tidy follows.)
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If you modify an authority and clear a specific subfield, you expect that
merge respects your edit and clears this subfield too in the biblio
records. It does in the new strict mode, but it does not yet in the
default loose mode.
This patch fixes that by adjusting the code around $exclude so that it
uses a new hash skip_subfields, built from the reporting tags from the old
and the new authority record.
This is supported again by some changes in the unit test.
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Originally aimed for 9988, adjusted for this report.
Old behavior was: pick the first tag. This is definitely wrong.
If you (would) merge 610 to 611, you don't want to get a 111.
This patch resolves the problem by determining the new tag in a small
helper routine _merge_newtag, and corrects the position of the new field
in the MARC record with append_fields_ordered. Too bad that MARC::Record
does not have such a function; it looks like insert_fields_ordered, but
it is different in case of multiple fields with the same tag.
Note: These two small helper functions are not tested separately, since they
should not be called outside of merge. They are implicitly tested by the
adjusted tests in Merge.t.
Note: In adding tests for this fix, I chose to simplify compare_field_count
(no need for the pass parameter), and replace the pass parameter of sub
compare_field_order by an exclude parameter, a hash of fields to exclude in
counting fields.
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Original fix from a patch on bug 11315.
Amended by Marcel de Rooy January 2017.
Test plan:
If you set mode to loose, the test will still pass.
If you set mode to strict, one test will fail. (Fixed later.)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Original fix from a patch on bug 5572.
Amended by Marcel de Rooy January 2017.
Note: This does not yet resolve the field order when merging to another
auth type, but is a good start.
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The marc subfield structure is currently cached using a global variable
of C4::Context. The infos are retrieved every time a new context is
created.
This patch suggests to use Koha::Cache instead.
To achieve this goal, a new subroutine is created
C4::Biblio::GetMarcSubfieldStructure, it will be called from code which
needs to get the marc subfield structure. GetMarcFromKohaField,
GetMarcSubfieldStructureFromKohaField, TransformKohaToMarc and
_get_inverted_marc_field_map are modified accordingly and the cache is cleared
when the table is updated (from the 3 pl scripts modified by this patch).
The caching done in C4::Context (marcfromkohafield) is removed.
Test plan:
Play with the marc subfield structure (in the administration module),
then add and edit records and make sure everything went fine.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Everything works as expected on my functional tests. I'm really happy to see the
patch introduces relevant tests for previously untested functions.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
subroutines should not take $dbh in parameter.
C4::Biblio::TransformMarcToKoha has it and does not use it.
Test plan:
Look at the patch and confirm that all occurrences of
TransformMarcToKoha have been modified.
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
This patch fixes a regression introduced by
commit 7e70202d34
Bug 15381: Remove GetAuthType and GetAuthTypeCode
This first version will reintroduce the same behavior as before bug
15381: the record will be displayed even if it's no in the DB
Test plan:
Search for authorities
delete one
The zebra's index is not updated yet and the results will contain the
record you have deleted.
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
perl -p -i -e 's/^.*set the version for version checking.*\n//' **/*.pm
+ manual adjustements
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Mainly a
perl -p -i -e 's/^.*3.07.00.049.*\n//' **/*.pm
Then some adjustements
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
perl -p -i -e 's/^(use vars .*)\$VERSION\s?(.*)/$1$2/' **/*.pm
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Bug 8064 (Merge several biblio records) change some code used in both
biblios and authorities merge tool without updating the authorities
merge tool.
This patch fixes that.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
The test t/db_dependent/AuthoritiesMarc.t fails because one test expect
the BuildSummary subroutine to not crash if the authtypecode passed in
parameter does not exist in the DB.
Test plan:
prove t/db_dependent/AuthoritiesMarc.t
should return green.
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Test fails before patch, and doesn't anymore after.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Test this patch with the previous one.
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
These 2 subroutines are now not necessary anymore, we can use
Koha::Authority::Types to retrieve the authority types.
This patch won't be easy to test. Indeed, a lot of file are updated.
To test it I would suggest to focus on the merge authority
You should also confirm that the authtypecode and the authtypetext
values are correctly displayed when navigating in the authority module.
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
There are 3 place where the auth_types table were requested directly
from the script or a subroutine. These 3 occurrences are easy to
replace with the new module.
Test plan:
1/ Search for authorities and use the "did you mean" feature for
authorities.
Focus on the authtypecode
2/ Edit authority and biblio frameworks.
The "Thesaurus" dropdown list should be correctly populated (UNIMARC
only).
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
The existing Koha::Authority does not use Koha::Objects and does not
exclusively deal with DB objects.
It makes much sense to move it to let the space free for a
Koha::Authority and Koha::Authorities modules based on Koha::Object.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
Signed-off-by: Frederic Demians <f.demians@tamil.fr>
Fixes a trivial coding error.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Most of them were found and fixed using codespell.
Fix also some related grammar issues.
In C4/Serials.pm a variable was renamed to make future codespelling
checks easier.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
http://bugs.koha-community.org/show_bug.cgi?id=14383
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This match sets $sortby (previously undefined value) as an empty string to get rid of the warns.
To test:
1) Go to a URL such as http://localhost:8080/cgi-bin/koha/opac-authorities-home.pl?op=do_search&type=opac&operator=contains&value=a&marclist=any&and_or=and
2) Notice the warns in the error log
3) Apply patch
4) Reload URL
5) Notice page still works but no warns in error log
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
NOTE: I would have done $sortby //= '';
But this works too. :)
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
The problem is the template in authority type summary is not respected
at all. It is only read to see which fields and subfields should appear
in the summary.
This patch fixes that.
It also fixes a bug in auth_finder.pl plugin when summary contains
fields other than 2XX.
Test plan:
0/ You must use a UNIMARC setup for those tests
1/ edit an authority type summary with:
NP : [200a][, 200b][ 200d][-- 152b --][ ; 200c][ (200f)] [001*] [ppn: 009*]
2/ create a new authority with previous fields (it is possible some
fields don't exist).
3/ search this authority and verify the summary is someting like:
NP : Name, D.-- NP -- 23849 ppn: my_ppn
4/ Verify some summary for existing authorities and check they are
correct.
5/ Edit a biblio record and use the plugin auth_finder.pl (for example
in a 7XX field)
6/ Do a search and verify the summary is correct
7/ Click on 'choose' or one of the numbered links ('1', '2', ... ; you
should have multiple 2XX fields for the numbered links to show up)
8/ Verify that the biblio field is correctly filled.
/!\ For the ppn, it should be defined in the zebra indexes.
In MARC21 and NORMARC setups, this patch should change nothing, please
verify that too (you can check that the auth_finder.pl plugin is still
working and the auth type summary is correctly displayed in authorities
search and auth_finder.pl plugin).
Signed-off-by: Frederic Demians <f.demians@tamil.fr>
It works as described, both in authority search result page, and in authority
data entry plugin.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Chris Nighswonger <cnighswonger@foundations.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
http://bugs.koha-community.org/show_bug.cgi?id=9987
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Completely get rid of warns.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
To test
Hit a url like
cgi-bin/koha/opac-authorities-home.pl?op=do_search&type=opac&operatorc=contains&marclistc=mainentry&and_orc=and&orderby=HeadingAsc&value=FIRECLAY
Notice the warns in the error log
Apply patch
Reload the url
Still works but no warns
(cherry picked from commit c98d805e490a82b2ea9f3d4e0f7278640942ba6d)
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Reformatted to fix error M Tompsett spotted
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Instead of 10 warnings, I now have only 4 warnings.
We're going in the right direction.
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
A compile time warning was being generated because \ does not quote
{ Replace the plethora of \ before meta and presumed meta characters
by \Q \E which gets perl to generate all the necessary escapes, we can
assume it knows its regex engine as well or better than the human
programmer
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Verified change reading the perldoc and also checked that
./misc/link_bibs_to_authorities.pl is no longer showing
the warn and still works.
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Works as described, running:
prove t/db_dependent/AuthoritiesMarc.t
doesn't show the warning anymore.
Test Plan:
1) Apply patch
2) Run updatedatabase.pl
3) View some records with authorities
4) Note your previously set authority separator should still be in use
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No koha-qa errors.
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch restores the display of the authority type summary for
MARC21, where at present the heading type (i.e., "Topical Term",
"Personal Names") come over for display in the template.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch corrects a small bug :
Authorities search on all types does not show summary because it is computed
with selected type (which is empty) instead of using found authority type.
Test plan :
- Go to intranet autorities search
- Perform a search on all authorities types
- Look at results
=> Without this patch, results rows do not display the autority summary, only
authorized headings
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
From a biblio record, if one wants to add a 600$a information, a pop-up
appears. On this new window, on search terms typed and validated, a table
result is displayed, with a column "Get It!" allowing the selection of an
authority. From here, different cases:
1) If we have a simple authority with 200$a and 200$b subfields, a link
"choose" is displayed, working correctly.
2) If the authority has different occurences of 200$a/200$b, numeric links (1 2
and so on) are displayed, one for each occurence. In the example of my
screenshot, the line with a "Paul, Korky -- Pauline, Korkette" summary
possesses two links : "1" will add "Paul, Korky" whereas "2" will add
"Pauline, Korkette" (couldn't come up with a better name ;)).
3) If the authority has 200$x or 200$y subfields defined, several links are
also created, when it should not be the case. In our example, "Niclausse,
Paul -- Expositions" will create a link "1" for "Niclausse, Paul" and a link
"2" for "Expositions". Clicking on the 2nd link leads to the following
error: Software error: Can't call method "subfields" on an undefined value
at
/home/asaurat/workspace/versions/community/authorities/blinddetail-biblio-search.pl
line 86. Only the cases 1 and 2 should be handled. The creation of links
for subfields like 200$x or 200$y should be removed.
This problem is caused by the use of " -- " has separator of authorities with
several headings, but also in some heading between main part and subdivisions.
This patch corrects this by using an array in authorities summary so that
presentation is computed in template. I've choosen to use the pipe separator
between authorities with several headings. This may be changed to be
configurable.
Test plan :
- Edit an authority type summary : for example subject (heading on 250) :
summary "[250a][ -- 250x]"
- Create an authority A1 with one heading and a subdivision : for example a
subject : 250$a "History" 250$x "20th century"
- Create an authority A2 with several headings. for example a subject : 250$a
"History" 250$a "Legends"
- Rebuild Zebra queue
- Go to OPAC and click on "Authority search" and search on "History"
=> You will find A1 and A2 :
History -- 20th century
History | Legends
- Go to intranet autorities search and search on "History"
=> You will find A1 and A2 :
History -- 20th century
History | Legends
- Edit a record using this autorities type as thesaurus : for example on 606$a
- Click on thesaurus link and search on "History"
=> You will find A1 and A2 :
History -- 20th century ; 0 times ; choose ; Edit authority
History | Legends ; 0 times ; 1 2 ; Edit authority
- Click on link "2" to chosse "Legends"
=> You get "Legends" in heading field : for example 606$a
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
I can confirm the problem and the solution. I have tested the patch on a large
DB with authorities having multiples headings. There is no regression on bug
4838.
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Without the patch I couldn't choose between multiple headings
in the authority plugin, but with the patch it works as described.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch puts the MARC21 variant of the bugfix in alignment
with the UNIMARC variant, removing the use of unnecessary
temporary variables.
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
I got the same warning for my UNIMARC DB.
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This was discovered when someone triggered an authority search
on an authority record that was missing what is assumed the
default subfield for a given field.
It, however, also can be triggered in an OPAC authority search
by looking at the record that lacks the default subfield for a
given field.
TEST PLAN
---------
1) Create an authority record with 180$x and NOT 180$v.
See C4::AuthoritiesMarc::BuildSummary in the 1.. foreach loop
for known tags and default values. The default subfields are
the first letter of the $subfields_to_report string.
2) Trigger the bug:
Method 1: /cgi-bin/koha/opac-authoritiesdetail.pl?authid=#
Where # is the authority id of your tweaked record.
The error occurs in Normal view.
Method 2: Home -> Cataloging -> + New record
-> Click the 'Tag Editor' on 100$a
Editing of $a to $b and back may be required.
3) Notice there is an error log entry.
4) Apply the patch
5) Attempt to trigger the bug again
6) That specific error log entry is not generated.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Could generate the warning with a missing 151$a with both methods.
No warning anymore after applying this patch.
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
This patch makes Koha <-> Zebra use MARCXML for the serialization when
using DOM, and USMARC for GRS-1.
* The following functions are modified to set the Zebra record syntax
according to the current sysprefs and configuration:
- C4::Context->Zconn
- C4::Context-_new_Zconn
* A new function 'new_record_from_zebra' is introduced, which checks the
context we are in, and creates the MARC::Record object using the right
constructor.
The following packages get touched to make use of the new function:
- C4::Search
- C4::AuthoritiesMarc
and the same happens to the UI scripts that make use of them (both in
the OPAC and STAFF interfaces).
* Calls to the unsafe ZOOM::Record->render()[1] method are removed.
Due to this last change the code for building facets was rewritten. And
for performance on the facets creation I pushed higher version
dependencies for MARC::File::XML and MARC::Record (we rely on
MARC::Field->as_string).
* Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc
are wrapped with eval for catching problems [2].
* As of bug 3087, UNIMARC uses the 'unimarc' record syntax. this case is
correctly handled.
* As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like:
- bib_index_mode (defaults to 'grs1' if not specified)
- auth_index_mode (defaults to 'dom')
here we do exactly the same.
To test:
- prove t/db_dependent/Search.t should pass.
- Searching should remain functional.
- Indexing and searching for a big record should work (that's what the
unit tests do).
- Test an index scan search (on the staff interface):
Search > More options > Check "Scan indexes".
- Enable 'itemBarcodeFallbackSearch' and try to circulate any word, it
shouldn't break.
- Searching for a biblio in a new subscription shouldn't break.
- Running bulkmarcimport.pl shouldn't break.
- And so on... for the rest of the .pl files.
[1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render()
[2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684)
Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
