Commit graph

2173 commits

Author SHA1 Message Date
b4c66d5cc4 Bug 17935: Adjust some POD lines, fix a few typos
This patch does the following:

[1] Move some POD lines from Cache to Caches.
[2] Correct C4::Plugins to Koha::Plugins in POD line of Koha::Plugins
[3] POD Koha/AuthorisedValue.pm: lib_opac moved to opac_description
[4] The POD in Koha/Patron.pm uses head2 and head3 inconsistently.
    Ran s/^=head2/=head3/ on those lines (7 substitutions on 7 lines)
[5] Correct a copied POD line from reports/issues_stats.pl in
    reports/reserve_stats.pl.
[6] Correct a test description in t/db_dependent/Koha/Authorities.t.
    You should never delete the library :)
[7] Correct typo shouild in a comment of rebuild_zebra.pl

Test plan:
[1] Read the patch. Does it make sense?
[2] Run perldoc Koha/Cache.pm and Koha/Caches.pm
[3] Run t/db_dependent/Koha/Authorities.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Cook <dcook@prosentient.com.au>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 14:12:50 +00:00
460c9b1b15 Bug 17982: Expose wrong use of uniq
This test should fail without the patch fixing the uniq calls.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 14:09:52 +00:00
2e315c0cc6 Bug 18089: Remove warnings from tests using DBIx::Class fixtures
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 14:07:02 +00:00
99cc3bdd0e Bug 18089 - Unit test
Signed-off-by: Jenny Schmidt <jschmidt@switchinc.org>

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 14:07:01 +00:00
Julian Maurice
bcd566c5f6 Bug 18014: QA follow-up: Use ok() instead of is() for '>' comparison
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 14:01:11 +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
24ac8392b3 Bug 18014: Add test to AuthoritiesMarc.t to expose problem in AddAuthority
Since AddAuthority uses max(id)+1 logic to produce the next authid,
authority id's will be reused when you delete the last record. This may
be a source of problems and will be addressed on the next patch.

This patch add a test to expose the problem.

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

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:10 +00:00
ac004aa63e Bug 18014: General update of AuthoritiesMarc.t
Modern::Perl, Koha::Database, etc.

Test plan:
Run t/db_dependent/AuthoritiesMarc.t

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:10 +00:00
add1579922 Bug 18009 - IssueSlip.t test fails if launched between 00:00 and 00:59
to test patch...

1/ set date between 00:00 and 00:59
$ sudo date -s 'Sun Jan 29 00:41:55 NZDT 2017'

2/ run prove, see fail
$ prove -v t/db_dependent/Members/IssueSlip.t
...
t/db_dependent/Members/IssueSlip.t (Wstat: 65280 Tests: 1 Failed: 0)
Result: FAIL

3/ apply patch

4/ run prove, see pass
$ prove -v t/db_dependent/Members/IssueSlip.t
...
All tests successful.
Result: PASS

NOTE: for code obscurity you could have also done a modulus 24. ;)

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 13:59:23 +00:00
137cbd8d09 Bug 17512: Improve handling dates in C4::Items
This is a follow-up on the internal server error on 0000-00-00 in the items
column onloan. This patch deals with preventing to have such dates at all
in the date fields of items.

It is accomplished by:
[1] Adding a (private) subroutine _mod_item_dates. It takes an item hash
    and replaces date values if needed.
[2] AddItem and ModItem call _koha_new_item resp. koha_modify_item. In these
    routines a call to the new _mod_item_dates is inserted.
[3] Although the routine is actually private, I have added some unit tests
    to Items.t.

Test plan:
[1] Add a new item. Fill a correct date in dateaccessioned and an invalid
    date in Price effective from (=replacementpricedate).
[2] Verify that dateaccessioned is saved correctly and replacementpricedate
    is still null (does not contain 0000-00-00).
[3] Edit the item again. Fill some text in dateaccessioned and put a correct
    date in replacementpricedate. Verify the results.
[4] Run t/db_dependent/Items.t

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-14 13:57:49 +00:00
185256deba Bug 17782: Fix tests
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-07 17:54:58 +00:00
b68557148d Bug 17782 - (QA Followup)
Fix tests

prove t/db_dependent/Koha/Patrons.t

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-07 17:54:58 +00:00
Lari Taskula
21ac9fcdc2 Bug 16387: Fix default shortened loan period time
When a loan period is shortened due to using decreaseLoanHighHolds* the time is
always set to the current time in X days, even if the original loan period is
given in days and not in hours.

It should default to 23:59 as is normal for loan periods given in days.

As original due date time defaults to 23:59 when given in days, this patch
modifies the hours and minutes of shortened due date to be equal to original due
date.

To test:
1. prove t/db_dependent/DecreaseLoanHighHolds.t

Signed-off-by: Grace McKenzie <grace.mcky@gmail.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-07 17:54:21 +00:00
80a43833f8 Bug 18044: Add a test
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-07 17:46:00 +00:00
eee1f23bc4 Bug 15030: Add tests
This test will prevent regression on the lost of data when
items.itemcallnumber is linked with a plugin.

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-07 17:43:49 +00:00
Olli-Antti Kivilahti
4f178fad53 Bug 17255 - Upgrade Elastic Search code to work with version 5.1
Builds on top of commit:
    Bug 17255 - Upgrade Elastic Search code to work with version 2.4+ - rebased wip

-Fix data type 'string' to 'keyword' and 'text'
-index: not_analyzed deprecated, replaced with type: keyword which is equivalent
-store: yes was deprecated, use store: true

TODO: Installer bindings to both the debian package install and the raw developer install.
A taster in Buug 17851

ZE TEST PLAN

0. Remove existing ES and reinstall ES 5.1
   apt-get purge elasticsearch
   Follow instruction here:
   https://www.elastic.co/guide/en/elasticsearch/reference/current/deb.html

1. Reset Elasticsearch index since facets are hard coded to dynamic search_marc_mappings.
1a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings();'
1b. If you get trouble, simply DELETE FROM [search_fields|search_marc_to_fields|search_marc_mapping];
    and retry 1a.
1c. Destroy elasticsearch index
    curl -XDELETE localhost:9200/koha_biblios
    so it can be recreated

2. Recreate the index:
   perl misc/search_tools/rebuild_elastic_search.pl
2a. Add something to index if your koha.biblio-table is empty

3. Fetch all indexed records and the facet for subject__facet

curl -XGET localhost:9200/koha_biblios/data/_search?pretty -d '{
  "aggregations": {
    "my_agg": {
      "terms": {
        "field": "subject__facet"
      }
    }
  }
}'

4. Run the included tests:
   perl t/db_dependent/Koha_Elasticsearch_Indexer.t
   perl t/db_dependent/Koha_Elasticsearch.t
   perl t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t

Have fun with your new ES 5.1 cluster!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-07 17:17:54 +00:00
cbbe3619b9 Bug 17960: Add opac_news.content values in tests
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-06 17:42:13 +00:00
a233f06574 Bug 17960: Rename opac_news.new with opac_news.content
The field opac_news.new is very confusing and should be renamed.
If you want to access it via Koha::NewsItem you will have trouble:

  use Koha::News;
  my $news_item = Koha::News->next;
  say $news_item->new;

=> Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78.

This patchset is going to rename this DB field to opac_news_content instead.

Since the opac_news.new can be used in notice templates, we need to warn the
user during the update DB process that some templates must be updated.

Test plan:
0/ Apply the first patch "Add a test to highlight the issue" and confirm that
the test fail
1/ Apply this second patch
2/ Execute the DB entry
3/ Confirm that you get a warning if at least one of your notice templates is
using opac_news.new
4/ Confirm that the test new pass
5/ Add/update and delete a news
6/ Confirm that the RSS new feed still works as expected

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-06 17:42:12 +00:00
a6b13fbcb3 Bug 17960: Add a test to highlight the issue
With only this patch applied, proving t/db_dependent/Koha/News.t
will return
  "Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78."

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-02-06 17:42:12 +00:00
42460b8714 Bug 17900: Update the tests to the new API
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:19:56 +00:00
a70980d825 Bug 17900: Fix possible SQL injection in patron cards template editing
To recreate:
/cgi-bin/koha/patroncards/edit-template.pl?op=edit&element_id=23%20and%201%3d2+union+all+select+1,user(),@@version+--%20

Look at the Profile dropdown list.

To fix this problem and to make sure it does not appears anywhere else
in the label and patroncards modules, I have refactored the way the
queries are built in C4::Creators::Lib
Now all of the subroutine takes a hashref in parameters with a 'fields'
and 'filters' parameters.
From these 2 parameters the new internal subroutine _build_query will
build the query and use placeholders.

Test plan:
1/ Make sure you do not recreate the vulnerability with this patch
applied.
2/ With decent data in the labels and patroncards modules, compare all
the different view (undef the New and Manage button groups) with and
without this patch applied.
=> You should not see any differences.

This vulnerability has been reported by MDSec.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-30 11:19:55 +00:00
90f9a3c6ac Bug 17588: get_account_lines->get_balance has been replace with account->balance
On previous bugs

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 14:25:35 +00:00
45cee0cec8 Bug 17588: Koha::Patrons - Move GetMemberIssuesAndFines
The GetMemberIssuesAndFines subroutine used to retrieve the issues,
overdues and fines for a given patron. Most of the time, only 1 or 2 of
these values were used.
This patch removes this subroutine and uses the new get_issues,
get_overdues and get_balance method from Koha::Patron and Koha::Account::Lines.

Test plan:
1/ Add overdues, issues and fines to different patrons
2/ On the checkout, checkin and patron search result and the patron
detail pages, these 3 informations, if displayed before this patch, must be
correctly displayed.
3/ Use the batch patron deletion tool and make sure that patrons with a
balance > 0 are not deleted

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 14:25:34 +00:00
e429db4746 Bug 17501: [Follow-up] QA Requests
This patch makes the following changes, as requested by QA:

[1] UploadedFile->delete always calls SUPER::delete.
    The return value normally comes from SUPER::delete; if removing the
    file failed, we return false. Two warns are kept.
    Since delete does no longer return the filename, a few changes were
    needed in tools/upload.pl.
[2] Method getCategories is moved to UploadedFiles. Script tools/upload.pl
    now only contains one call. Added a use C4::Koha.
[3] Calls UploadedFiles->delete as class method. As a result I removed
    method delete_errors for now; may be reconsidered on a new report.
[4] Adjusted three ->search calls for id and public to ->find calls.
[5] If you pass no id to upload.pl when deleting, you don't get an alert.

All by all, we got rid of 15 lines !

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

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2017-01-20 14:20:07 +00:00
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
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
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
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
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
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
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
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
41358176e1 Bug 17726: TestBuilder - Add default values
The items.more_subfields_xml is set to random data (generated by
TestBuilder), and so GetMarcBiblio does not manage to embed items (if
needed).

The error is:
  :1: parser error : Start tag expected, '<' not found

More precisely it explodes in
C4::Items::_parse_unlinked_item_subfields_from_xml when
MARC::Record->new_from_xml is called with an invalid xml

This patch adds a default values mechanism to TestBuilder to avoid
modifying all the existing calls.

Test plan:
Set SearchEngine to ElasticSearch
prove t/db_dependent/Circulation.pl
should return green with this patch

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:17 +00:00
11dfb2e0b2 Bug 8361: Do not allow checkouts if no rules are defined
We should require a circulation rule to allow checkouts and reject them
if no rules are defined.

Test plan:
- Delete all issuing rules
- Check an item out
=> Without this patch the checkout is allowed
=> With this patch applied it is rejected

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:41:59 +00:00