Commit graph

223 commits

Author SHA1 Message Date
d49e8bd5e2 Bug 9988: Few subtle changes for postponed merge
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>
2017-04-13 08:53:47 -04:00
04263912c7 Bug 9988: Check the merge limit in sub merge
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>
2017-04-13 08:53:46 -04:00
06647d29a7 Bug 9988: Remove the Zebra code from sub merge
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>
2017-04-13 08:53:46 -04:00
4c762ba69c Bug 9988: Merge should have a parameter hash
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>
2017-04-13 08:53:46 -04:00
fd8c2d4e35 Bug 18070: Correct error from podchecker on line 1376
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>
2017-03-03 18:12:10 +00:00
17e84750d9 Bug 18070: [Follow-up] What should DelAuthority return?
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>
2017-03-03 18:12:09 +00:00
cb58b11f67 Bug 18070: Extend sub merge to remove fields for deleted authorities
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>
2017-03-03 18:12:09 +00:00
ad38b24308 Bug 18014: AddAuthority should respect AUTO_INCREMENT
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>
2017-02-14 14:01:11 +00:00
6ddd4b2f5f Bug 17913: [Follow-up] Fix duplicate $9s after merging in loose mode
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>
2017-01-20 13:55:13 +00:00
dd1c63230f Bug 17913: Run perltidy on the inner foreach loop
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>
2017-01-20 13:55:12 +00:00
28b74224d3 Bug 17913: We always need some housekeeping
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>
2017-01-20 13:55:12 +00:00
ede08d725e Bug 17913: Remove possible duplicates in strict merge mode
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>
2017-01-20 13:55:12 +00:00
681ae8430e Bug 17913: Do not keep a cleared subfield in loose merge mode
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>
2017-01-20 13:55:11 +00:00
8cde85451e Bug 17913: Fix the new field tag in merge when changing type
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>
2017-01-20 13:55:11 +00:00
Frédérick
79c81920ab Bug 17913: Use AuthorityMergeMode pref in sub merge
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>
2017-01-20 13:55:10 +00:00
ffe1f6555c Bug 17913: Use replace_with instead of insert_grouped_field
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>
2017-01-20 13:55:10 +00:00
172ebfe520 Bug 16431: Use Koha::Cache to cache marc subfield structure
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>
2016-07-08 12:44:58 +00:00
Jesse Weaver
37ea74669c Bug 12748: (QA followup) fix several tiny QA tools warnings
* admin/searchengine/elasticsearch/mappings.yaml:
    * Fix indentation so QA tools believe it's valid YAML
  * C4::AuthoritiesMarc: Remove tab character
  * C4::Search: Fix POD
  * Koha::BiblioUtils: remove "return undef"
  * Koha::Schema::Result::Search*: rename type to _type
  * Koha::SE::ES::Search:
    * Fix spelling/@_[0]
    * Add my to loop variable
    * Remove unused var $error
    * Add use Modern::Perl
    * Fix undefined variables
  * Koha::ES::Indexer: fix POD
  * Koha::SE::ES::QueryBuilder:
    * Don't define variables in conditionals
    * Fix POD
  * Koha::SE::Zebra::Search:
    * Fix debug warns/POD/barewords
    * Koha::SE::Zebra::Search: add use Modern::Perl
  * kohastructure.sql: remove old, unused elasticsearch_mappings table,
    add new tables
  * labels/label-item-search.pl: remove C4::Dates
  * opac/elasticsearch.pl: remove old file

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>
2016-04-26 20:20:13 +00:00
Robin Sheat
b4392018bc Bug 12478: make things using SimpleSearch use the new version
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>
2016-04-26 20:20:07 +00:00
c564f30155 Bug 16169: Change prototype for C4::Biblio::TransformMarcToKoha
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
2016-04-07 00:04:21 +00:00
2d961b5ad4 Bug 16056: Do not crash when searching for an authority if zebra's index is not up-to-date
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>
2016-04-01 19:04:38 +00:00
798d38e4c7 Bug 16011: $VERSION - Remove comments
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>
2016-03-24 17:20:29 +00:00
017699c345 Bug 16011: $VERSION - Remove the $VERSION init
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>
2016-03-24 17:20:28 +00:00
3830d78d46 Bug 16011: $VERSION - remove use vars $VERSION
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>
2016-03-24 17:20:26 +00:00
Julian Maurice
5ef5fb5617 Bug 15358: Fix authorities merge
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
2016-02-03 23:03:33 +00:00
72f96d3e3b Bug 15467: If authtypecode is not defined, do not explode
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>
2016-01-07 19:03:27 +00:00
4a34fd2fd3 Bug 15381: Fix error in SearchAuthorities when no authtypecode
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Brendan Gallagher brendan@bywatersolutions.com
2015-12-31 18:59:03 +00:00
7e70202d34 Bug 15381: Remove GetAuthType and GetAuthTypeCode
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
2015-12-31 18:59:02 +00:00
7283069a9d Bug 15381: Remove getauthtypes and getauthtype
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
2015-12-31 18:59:02 +00:00
e28d54f7d9 Bug 15381: Use Koha::Authority::Types in some other places
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
2015-12-31 18:59:02 +00:00
03c89fa02e Bug 15380: Move Koha::Authority to Koha::MetadataRecord::Authority
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
2015-12-31 18:50:41 +00:00
Lyon3 Team
6c94dc7987 Bug 15130: useless unitialization warnings when updating authorities
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>
2015-11-06 12:08:51 -03:00
Stefan Weil
64925f7522 Bug 14383: C4: Fix some typos (mostly in comments and documentation)
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>
2015-06-22 17:34:45 -03:00
Aleisha
bf6ded93e8 Bug 14075: Undefined value creates noisy warns in C4::AuthoritiesMarc
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>
2015-05-06 15:26:04 -03:00
Jonathan Druart
927aedafa1 Bug 10985: [UNIMARC] Fix authority summary
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>
2015-04-22 11:54:36 -03:00
Jonathan Druart
a6c9bd0eb5 Bug 9978: Replace license header with the correct license (GPLv3+)
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>
2015-04-20 09:59:38 -03:00
Jonathan Druart
acff264558 Bug 12861: (follow-up) Noisy warn in error logs
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>
2015-01-21 10:47:47 -03:00
Chris Cormack
b2ce6347e5 Bug 12861 : Noisy warn in the error logs
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>
2015-01-21 10:47:41 -03:00
Jonathan Druart
df74fe9be8 Bug 11944: FIX Error on editing an authority
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
2015-01-13 13:07:11 -03:00
Colin Campbell
61440fab13 Bug 12654 Correct incorrectly quoted regexp
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.
2014-07-30 11:06:27 -03:00
ad6d367763 Bug 10330 - Rename system preference authoritysep to AuthoritySeperator
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>
2014-05-23 13:26:48 +00:00
Galen Charlton
9fe024667f Bug 7442: (follow-up) restore display of heading type for non-UNIMARC
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>
2014-05-05 01:27:33 +00:00
Fridolyn SOMERS
ebcb64e9fa Bug 7442 (follow-up) ensuring that summary is display when searching on all types of authorities
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>
2014-05-05 01:10:47 +00:00
Fridolyn SOMERS
99dbc5cdf7 Bug 7442: fix crash when selecting an authority with 200$x or 200$y (UNIMARC)
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>
2014-05-05 01:09:29 +00:00
Galen Charlton
cc7a99ba7f Bug 11797: (follow-up) fix MARC21 branch of bug more idiomatically
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>
2014-04-15 16:01:31 +00:00
Jonathan Druart
fa989631f6 Bug 11797: fix odd number of elements in hash (UNIMARC)
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>
2014-04-15 16:01:31 +00:00
Mark Tompsett
129bc32811 Bug 11797: fix odd number of elements in hash warning (MARC21)
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>
2014-04-15 16:01:31 +00:00
daf2ebc4f5 Bug 11096: support the retrieval of large MARCXML records
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>
2014-02-28 19:50:09 +00:00
Galen Charlton
27c312b721 Bug 11533: fix authority searching with no sorting when QueryParser is enabled
This patch fixes an issue where chosing 'None' as the sort order
for an authority search would result in zero hits if QueryParser is
eanbled.

This patch also adds some additional test cases.

To test:

[1] Enable QueryParser.
[2] Perform an authority search in the staff interface that
    uses 'Heading A-Z' as the sort order and returns hits.
[3] Run the same search, but with the sort order set to 'None'.
    No hits are returned.
[4] Apply the patch.
[5] Do step 3 again.  This time, hits should be returned.
[6] Verify that prove -v t/db_dependent/Search.t passes.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-01-24 14:02:48 +00:00
5885077fbb Bug 11473 - add 'biblio' and 'item' to cataloguing logging info
This patch adds the words 'biblio' and 'item' to the 'info'
of the cataloguing logs which were missing them (such as biblio
delete, biblio mod, item mod, upload cover image).

This patch also adds 'authority' for authority mod.

_TEST PLAN_

Before applying:
1) Create/view mods for items, biblios, and authorities.
2) Create/view biblio deletion
3) Create/view upload cover image log
4) Note that none of these contain the words 'biblio','item',or
'authority' in their "Info" columns.

Apply patch.

5) Repeat steps 1-3
6) Note that the new logs contain 'biblio','item', and 'authority'
in their "Info" column, while the past ones don't.
7) Note also that 'biblio' and 'item' will have 'Biblio' and 'Item'
appear in their "Object" column for the new logs

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
2014-01-17 05:42:44 +00:00