Commit graph

28303 commits

Author SHA1 Message Date
a118dd2ba0 Bug 17501: Additional polishing (POD, unit tests)
This patch adds some documentation lines.
And mainly rearrangs the tests in Upload.t. The 'basic CRUD testing' is
not needed separately any more. A new test catches the "file missing"
warn.

Test plan:
[1] Run perldoc on UploadedFile[s].pm
[2] Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:06 +00:00
3acee79310 Bug 17501: Rename Upload to Uploader
Why? Koha::Uploader now only contains the actual CGI upload. The new name
better reflects its handler status.
Pragmatically, the difference between Uploaded and Uploader makes it
easier to specifically search for them in the codebase.

Test plan:
[1] Run t/db_dependent/Upload.t.
[2] Add an upload via the interface.
[3] Check the code:
    git grep "Koha::Upload;"
    git grep "Koha::Upload\->"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:06 +00:00
ba5cd24553 Bug 17501: Move getCategories and httpheaders from Upload.pm
Class method getCategories has no strict binding to Upload.pm. While
Upload.pm is now restricted to the actual uploading process with CGI
hook, this routine fits better in the UploadedFile package.

Class method httpheaders can be moved as well for the same reason. Note
that it actually is an instance method. The parameter $name is dropped.

Test plan:
[1] Run t/db_dependent/Upload.t.
[2] Check the categories in the combo box of tools/upload.
[3] Check a download via tools/upload and opac-retrieve-file.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:05 +00:00
158442eb9e Bug 17501: Remove Koha::Upload::get from Koha::Upload
The get routine actually returns records from uploaded_files. It should be
possible to replace its calls by direct calls of Koha::UploadedFiles.

This patch is the crux of this patch set. It deals with all scripts that
use Koha::Upload.

In the process we do:
[1] Add a file_handle method to Koha::UploadedFile. This was previously
    arranged via the fh parameter of get.
[2] Add a full_path method to UploadedFile. Previously returned in the
    path hash key of get. (Name is replaced by filename.)
[3] Add a search_term method too (implementing get({ term => .. }).
    This logic came from _lookup.
[4] Add a keep_file parameter to delete method. Only used in test now.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Go to Tools/Upload. Add an upload, download and delete.
[3] Add another public upload , search for it.
    Use the hashvalue to download via opac with URL:
        cgi-bin/koha/opac-retrieve-file.pl?id=[hashvalue]
[4] Go to Tools/Stage MARC for import. Import a marc file.
[5] Go to Tools/Upload local cover image. Import an image file.
    Enable OPACLocalCoverImages to see result.
[6] Test uploading a offline circulation file:
    Enable AllowOfflineCirculation, and create a koc file (plain text):
    Line1: Version=1.0\tA=1\tB=2
    Line2: 2016-11-23 16:00:00 345\treturn\t[barcode]
    Note: Replace tabs and barcode. The number of tabs is essential!
    Checkout the item with your barcode.
    Go to Circulation/Offline circulation file upload.
    Upload and click Apply directly.
    Checkout again. Repeat Offline circulation file upload.
    Now click Add to offline circulation queue.
[7] Connect the upload plugin to field 856$u.
    Enable HTML5MediaEnabled.
    Upload a webm file via the plugin. Click Choose to save the URL,
    and put 'video/webm' into 856$q. Save the biblio record.
    Check if you see the media tab with player on staff detail.
    (See also: Bug 17673 about empty OPACBaseURL.)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:05 +00:00
7a62005468 Bug 17501: Move Koha::Upload::delete to Koha::UploadedFile[s]
Since delete is not part of the upload process, we will move it now
to Koha::UploadedFile[s].
Deleting the file will be done in UploadedFile.
The (multiple) delete method in UploadedFiles refers to the single delete.

Test plan:
[1] Run t/db_dependent/Upload.t
    The warning ("but file was missing") in the last subtest is fine;
    the file did not exist. Will be addressed in a follow-up.
[2] Search for uploads on Tools/Upload. Clone this tab (repeat search on
    a new tab in your browser).
[3] Delete an existing upload on the first tab.
[4] Try to delete it again on the second tab. Error message?
[5] Bonus points:
    Add an upload. Mark the file immutable with chattr +i. Try to delete
    the file. You should see a "Could not be deleted"-message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:04 +00:00
42f0bdf9f1 Bug 17501: Use Koha::Object in Koha::Upload::_delete
Note: This is the last occurrence where we use DBI to perform a CRUD
operation. In this case a delete from uploaded_files.

We now call Koha::UploadedFile[s]->delete to only delete the record
from the table. A next step will be moving the additional functionality
of removing the file(s) too.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Delete an upload from tools/upload.pl

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:04 +00:00
4efe9e82cd Bug 17501: Use Koha::Object in Koha::Upload::_lookup
The _lookup routine performs a few select statements on uploaded_files.
In this patch the SQL statements are replaced with Koha::Object calls.
One call of _lookup is replaced directly by Koha::UploadedFiles.

Note: _lookup can be removed in a later stage.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file in some upload category
[3] Try to upload the same file into the same category. Error?
[4] Try to upload the same file in another category. Should work.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:04 +00:00
44c4b3fca6 Bug 17501: Use Koha::Object in Koha::Upload::_register
The _register routine basically inserts a new record in uploaded_files.
It should use Koha::UploadedFile now.

Test plan:
[1] Run t/db_dependent/Upload.t
[2] Upload a file via Tools/Upload.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:03 +00:00
44d4dc7040 Bug 17501: Introduce Koha::Object[s] classes for UploadedFile(s)
In the next set of patches we will start using these new classes in
Koha::Upload, and scripts using it.
This is just the starting point of that migration.

Test plan:
[1] Run t/db_dependent/Upload.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:03 +00:00
57568b9a0b Bug 16733: [Follow-up] Add $home to api path too
In the meantime api was enabled in plack.psgi and needs a little tweak
too for a dev install.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:15:27 +00:00
08ece513cd Bug 16733: Adjust other debian scripts using PERL5LIB
This patch makes the following changes:

koha-foreach, koha-upgrade-schema (shell scripts):
[1] Read default file
[2] Include helper functions
[3] Add call to adjust_paths_dev_install
[4] Replace hardcoded path by $PERL5LIB

koha-shell (perl script):
[1] Remove hardcoded lib path
[2] Add a sub that reads PERL5LIB from default or koha-conf, just as the
    shell scripts do.

koha-plack (shell script), plack.psgi:
[1] Add call to adjust_paths_dev_install
[2] Remove hardcoded lib path
[3] Add installer path to PERL5LIB, remove it from plack.pgsi

koha-sitemap (shell script):
[1] Add call to adjust_paths_dev_install
[2] Remove hardcoded lib path
[3] Add installer path to PERL5LIB
[4] Adjust path for call to sitemap cron job

koha-start-sip (shell script):
[1] Read default file
[2] Include helper functions
[3] Add call to adjust_paths_dev_install
[4] Adjust path to C4/SIP

koha-stop-sip (shell script):
[1] Remove KOHA_CONF and PERL5LIB (not needed to stop the daemon)
[2] Same for paths in daemon client options

NOTE: Script debian/scripts/koha-upgrade-to-3.4 has been left out
intentionally.

Test plan:
[1] Regular install:
    Run koha-foreach echo Hi
    Run koha-upgrade-schema yourinstance
    Run koha-shell yourinstance
    If you have plack, run koha-plack --start|--stop yourinstance
    Run koha-sitemap --generate yourinstance
    Run koha-start-sip yourinstance
    Run koha-stop-sip yourinstance

[2] Dev install [yourinstance] with <dev_install> in koha-conf.xml:
    Run koha-upgrade-schema yourinstance
    Run koha-shell yourinstance
    If you have plack: koha-plack --start|--stop yourinstance
    Run koha-sitemap --generate yourinstance
    Run koha-start-sip yourinstance
    Run koha-stop-sip yourinstance

[3] Git grep on koha/lib
    You should no longer see occurrences in debian/scripts except:
    koha-translate: see report 16749
    koha-upgrade-to-3.4: left out intentionally

[4] Git grep on koha/bin
    You should only see hits for lines with koha-functions in the
    debian scripts except:
    koha-upgrade-to-3.4: left out intentionally

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Most scripts tested on Wheezy (although it would not matter much).
Plack script tested on Jessie.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:15:27 +00:00
2a7d595736 Bug 16733: Adjust koha-rebuild-zebra
[1] Add a call to the new adjust_paths_dev_install
[2] Differentiate location of rebuild_zebra.pl
[3] Replace a hardcoded path by $PERL5LIB

Test plan:
Adjust a biblio record in package or dev install.
Run koha-rebuild-zebra -b -z for same instance.
Verify that the change has been indexed.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:15:27 +00:00
2fc1b469e2 Bug 16733: Adjust koha-indexer
[1] Add a call to the new adjust_paths_dev_install
[2] Differentiate location of rebuild_zebra.pl

NOTE: The scripts assume koha-functions.sh to be in /usr/share/koha/bin.
Finding a better location for this shell library may be hard.

Test plan:
Run koha-indexer for a regular package install or a dev install.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:15:27 +00:00
513618e627 Bug 16733: Add adjust_paths_dev_install to koha-functions.sh
This new function checks koha-conf.xml for a given instance and if it
contains a dev_install line, it adjusts PERL5LIB and KOHA_HOME
accordingly. Otherwise it does not touch the values of these
variables as normally read from /etc/default/koha-common.

The function will be used in various debian scripts to allow for more
flexibility with dev installs. And at the same time aiming to make better
use of PERL5LIB and KOHA_HOME.

Test plan:
[1] Add <dev_install>/not/there</dev_install> to your koha-conf.xml.
[2] Run on the command line:
        PERL5LIB=test
        source [path-to-your-instance]/debian/scripts/koha-functions.sh
        echo $PERL5LIB
        adjust_paths_dev_install [name-of-your-instance]
        echo $PERL5LIB
    The last echo should be: /not/there
[3] Remove the <dev_install> line and repeat step 2.
    The last echo should be: test

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:15:26 +00:00
7d140258a0 Bug 7533: Add a warning to the about page if template_cache_dir is not set
We need to tell the administrators that it would be great for them to
set this config entry.

Test plan:
- Do not set template_cache_dir and confirm that you see the warning
- Set template_cache_dir and confirm that you do not see the warning

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Both templates for koha-conf.xml are updated. After applying the
patach a warning was correctly displayed. After adding
template_cache_dir to koha-conf.xml and restarting memcached it
went away.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:13:53 +00:00
819cea62f2 Bug 7533: Add the template_cache_dir entry to koha-conf.xml
And comment it, as we don't know what are the sysop's preferences

Signed-off-by: Magnus Enger <magnus@libriotech.no>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:13:52 +00:00
radiuscz
aaf6b6724f Bug 17487: Styling moved from style attribute into staff-global.css
Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header
4) [Optionally] Set some style in IntranetUserCSS for class z3950checks

https://bugs.koha-community.org/show_bug.cgi?id=17487

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:11:55 +00:00
radiuscz
f05b2986da Bug 17487: Links to "select/clear all" moved below the header tag
Test plan:
1) Apply patch
2) Display Z39.50 search dialogs:
   - cataloguing / new from Z39.50
   - authorities / new from Z39.50
   - acquisition / new from an external source
3) Select all / Clear all should be placed below "Search targets" header

https://bugs.koha-community.org/show_bug.cgi?id=17487

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:11:54 +00:00
1104e61635 Bug 17899 - Show only mine does not work in newordersuggestion.pl
Bug 12775 added a link "Show only mine" in newordersuggestion.pl.
This does not work, no results.

Also corrects the fact that click must not do default action by adding e.preventDefault().

Test plan :
- You must have suggestions you have accepted
- Create a new order from suggestion : /cgi-bin/koha/acqui/newordersuggestion.pl
- Click on Show only mine
=> Without patch the table is empty showing "No matching records found"
=> With patch you see only suggestions you have accpeted

Signed-off-by: Zoe Schoeler <crazy.mental.onion@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:10:36 +00:00
Mark Tompsett
7401d9422b Bug 17920: t/db_dependent/Sitemapper.t fails because of permissions
The directory it attempts to create an xml file may not be writable for
the user running the test. By changing the directory from the current
directory to a temporary one, the test runs. After all 'chmod 777
t/db_dependent' is a bad idea.

TEST PLAN
---------
1) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
   -- fails
2) apply patch
3) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
   -- succeeds
4) run koha qa test tools

Tested without qa tools
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:04:21 +00:00
9fe4f3b2ba Bug 17813 - DBRev 16.12.00.006
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:00:47 +00:00
0f3ced357d Bug 17813: (QA followup) Properly check DB structure before altering it
This patch makes the atomic update use the new functions introduced by
bug 17234 for checking the structure before attempting to call ALTER
TABLE.

It checks for the column existence, and also if it is a primary key.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:59:42 +00:00
48d7667af9 Bug 17813: DBIC update
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:59:41 +00:00
cb17720f47 Bug 17813: Add a primary key to borrower_attributes
This patch adds 'borrower_attributes' a field (if) which
will act as a primary key.

This is needed for DBIC to be used to handle rows, and also will help
when faced with the implementation of the REST api for this resource.

To test:
- Run all patron modification / attributes and verify nothing breaks
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:59:41 +00:00
07626b3b6e Bug 17913 - DBRev 16.12.00.005
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:58:52 +00:00
5a9ffa92ce Bug 17808: Fix behavior when editing a circ rule
The original behavior is broken, see https://stackoverflow.com/questions/21410484/jquery-selector-to-find-out-count-of-non-empty-inputs

Test plan:
Edit a circ rule
=> Without this patch you get a useless message
=> With this patch, no message
Edit a circ rule with content in inputs
=> With or without this patch you get a useful message

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:57:15 +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
8fcf2d6e50 Bug 17913: [Follow-up] Another small fix for UNIMARC
Adding another delete for field 100.
Will mock GetMarcBiblio on a new report.

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
mbeaulieu
9290e10abe Bug 17913: Adjust merge test for AuthorityMergeMode
Original fix from a patch on bug 11315.
Amended by Marcel de Rooy January 2017.

Test plan:
Run t/db_dependent/Authorities/Merge.t in both loose and strict mode.
Should no longer make a difference.

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
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
f9a4204ce3 Bug 17913: Add AuthorityMergeMode preference
Original patch from bug 5572, dating back to 2011!
Amended by Marcel de Rooy January 2017. Renamed the pref.

The fix on this report is based on this preference.
Depending on the pref, subfields will be deleted or kept.

Test plan:
Run the dbrev.

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:09 +00:00
Julian Maurice
e3a12e517a Bug 17909: QA followup: remove unused var and move global var
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:49:05 +00:00
9510b02330 Bug 17909: [Follow-up] Quick fix for UNIMARC
UNIMARC inserts field 100. This interferes the field count and order
in the test.
Note: This is a quick fix. Will polish it after bug 17913.

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:49:05 +00:00
b5a95937e6 Bug 17909: Followup - fix typos
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

EDIT:

Adjusted three small typos that did not disturb the test in its current
form, but do when we are fixing bugs on bug 17913.

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:49:04 +00:00
4457d2e64c Bug 17909: Additional polishing
No spectacular things:

[1] Move the framework modifications to a sub. Use same style to create auth types and linked fields.
[2] Change some new Object occurrences to Object->new.
[3] Add tests for field count and field order in the first two subsets.
[4] Few whitespace changes (sorry) and comment lines.

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:49:04 +00:00
a7c2fc7505 Bug 17909: Add tests for merging with another authtype
Originally aimed for bug 9988. Adjusted in line with other subtests.
Will polish the three subtests a little more on a follow-up.

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:49:04 +00:00
mbeaulieu
b89be0d72f Bug 17909: Adding tests from bug 11315
Based on original patch from Maxime Beaulieu on bug 11315.
Amended by Marcel de Rooy on report 17909.

EDIT:

Original tests have been adjusted in view of:
[1] Test on bug 11315 heavily leaned on DBD::Mock. Since we are
    using Test::DBIx::Class on such tests now, this would need attention.
    Moreover, the advantage of mocking the database in this case is at
    least arguable.
[2] Matching the first (somewhat older) subtest of 11700.
[3] Simplification and readability.
    Look e.g. at the use of $MARCto and $MARCfrom on 11315.

This made me merge them in the db_dependent counterpart.

Also note that this subtest adds another needed test case: the merge from
auth1 to modified auth1, while 11700 tested auth1 to auth2.

Test plan:
Just 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:49:04 +00:00
Julian Maurice
d4de65c21f Bug 17909: Add unit tests for C4::AuthoritiesMarc::merge
Original patch from Julian Maurice on bug 11700.
With sign offs by:
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Amended by Marcel de Rooy on report 17909.

EDIT (January 2017):
Removed some tests not related to merge.
Put remaining tests in a subtest, made them working on current merge.
Slightly revised the mocking.

Note: I plan to move the zebra retrieval stuff outside merge in one of
the next stages, and replace it by calling Koha::SearchEngine. This will
reduce mocking complexity here.

Test plan:
Just 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:49:03 +00:00
b732963e2f Bug 17880 - Use version.pm to parse version numbers in C4::Installer::PerlModules
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:47:27 +00:00
2321ae0d79 Bug 17880 - Add test to check version number comparison
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:47:26 +00:00
71740f98a8 Bug 17836: (ILSDI) Regression test
Sponsored-by: ByWater Solutions

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-01-20 13:44:57 +00:00
13923e7b05 Bug 17836: (ILSDI) Make GetPatronInfo fill 'charges' correctly
This trivial fix corrects a typo on C4/ILSDI/Services.pm.
It was hidden because the tests for ILSDI only cover the 'attributes'
portion of the response. I added regression tests for this.

To test:
- Have the regression test patch applied
- Run:
  $ prove t/db_dependent/ILSDI_Services.t
=> FAIL: Tests fail because 'charges' is always set to 1
- Apply the patch
- Run:
  $ prove t/db_dependent/ILSDI_Services.t
=> SUCCESS: Tests pass
- Sign off :-D

Sponsored-by: ByWater Solutions

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-01-20 13:44:56 +00:00
cd771f3686 Bug 17726: biblioitems.marc has been removed
We will certainly have to do something with the biblio_metadata.metadata
field later.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:43:18 +00:00
ddf1d9bcdd Bug 17726: [QA Follow-up] Add test descriptions
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 13:43:18 +00:00