Commit graph

6906 commits

Author SHA1 Message Date
2d1b80d216 Bug 21749: Fix import of CSV biblio framework
Bug 9701 added 2 new columns to the table marc_tag_structure which broke
the import of biblio frameworks if the CSV file was generated with a
previous version.

Test plan:
- Export a biblio framework from 17.11
- Import it into 18.05+

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

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-15 12:47:36 +00:00
9a011c58e7 Bug 21467: Do not erase the internal note of the first order
For the subscription we would like to keep the original internal note
(from the first order), to display it unmodified each time we receive
issues.

Sponsored-by: BULAC - http://www.bulac.fr/

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-13 19:31:10 +00:00
414aa0aef6 Bug 15836: Remove deprecated subroutines
It will take too much time to fix the failures here, given the
simplicity of the subroutines I would be in favor of deleting them and
continue the move on a separate bug report to use Koha::Object-objects

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I've checked that everything removed is no longer in use.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-13 18:56:18 +00:00
02a942767f Bug 15836: Mark C4::ClassSource subs DEPRECATED
These subs are no longer used from scripts and should be removed. It
should be done on a separate bug report given that additional work is
needed

Sponsored-by: Goethe-Institut

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-13 15:18:22 +00:00
65c4d8019e Bug 15836: Add the ability to defined custom methods to split call number in labels
Currently the call number splitting seems to be mostly implemented for
DDC and LC classifications.
Those are both not very common in some countries.
A lot of libraries use their own custom classification schemes so the call number
plitting is something that should be individually configurable.

This enhancement adds the ability to define custom splitting rules based
on regular expressions.

How does it work so far?
From C4/Labels/Label.pm there are 3 differents splitting methods defined, depending on items.cn_source.
  if cn_source is "lcc' or 'nlm' we split using Library::CallNumber::LC
  if cn_source is 'ddc' we split using a in-house method
Finally there is a fallback method to split on space
And nothing else is done for other cn_source

The idea of this patch is to mimick what was done for the "filing rules" and add
the ability to define "splitting rules" that will be used by the "Classification sources".
A classification source will then have:
  * a filing rule used to sort items by callnumbers
  * a splitting rule used to print labels

To acchieve this goal this enhancement will do the following
modifications at DB level:
* New table class_split_rules
* New column class_sources.class_split_rule

Test plan:
* Execute the update database entry to create the new table and
column.
I. UI Changes
a) Create/modify/delete a filing rule
b) Create/modify/delete a splitting rule
c) Create/modify/delete a classification source
=> A filing rule or splitting rule cannot be removed if used by a
classification source

II. Splitting rule using regular expressions
a) Create a splitting rule using the "Splitting routine" "RegEx"
b) Define several regular expressions, they will be applied one after
the other in the same order you define them.
Something like:
  s/\s/\n/g         # Break on spaces
  s/(\s?=)/\n=/g    # Break on = (unless it's done already)
  s/^(J|K)\n/$1 /   # Remove the first break if callnumber starts with J or K
c) You can test the regular expressions using filling the textarea with
a list of callnumbers. Then click "Test" and confirm the callnumbers are
split how you expected.
d) Finally create a new classification source that will use this new
splitting rule.

III. Print the label!
a) Create a layout. It should have the "Split call numbers" checkbox
ticked, and display itemcallnumber
b) Use this layout to export labels, use items with different
classification source ('lcc', 'ddc', but also the new one you have
create)
=> The callnumbers should have been split according to the regex you
defined earlier!

Notes:
* The update database entry fill the class_sources.class_split_rule
with the value of class_sources.class_sort_rule
If default rules exist it will not work, we should add a note in the
release notes (would be enough?)
* C4::ClassSplitRoutine::* should be moved to Koha::ClassSplitRule,
but it sounded better to keep the same pattern as ClassSortRoutines
* Should not we use a LONGTEXT for class_split_rules.split_regex instead
of VARCHAR(255)?

* class_sources.sql should be filled for other languages before pushed
to master!

IMPORTANT NOTES: The regular expressions are stored as it, and eval is
used to evaluate it (perlcritic raises a warning about it (Expression
form of "eval"). It can lead to serious security issues (execution of
arbitrary code on the server), especially if the modifier 'e' is used.
We could then remedy the situation with one of these following points:
- Assume that this DB data is safe (We can add a new permission?)
- Assume that the data is not safe and deal with possible attack
Cons: how be sure we are exhaustive? Making sure it matches ^s///[^e/]*$
would be enough?
- Use Template Toolkit syntax instead (Really safer?)
  [% callnumber.replace('\s', '\n').replace ... %]
- Cut the regex parts: find, replace, modifiers
like we already do for Marc modification template. Cons: we are going to
have escape problems, the "find" and "replace" parts should not be
handle the same way (think "\n", "\\n", "\1", "\s", etc.)
I did not manage to implement this one easily.

Sponsored-by: Goethe-Institut

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-13 15:18:22 +00:00
David Gustafsson
d659649a13 Bug 20356: Add EmailSMSSendDriverFromAddress system preference
Add EmailSMSSendDriverFromAddress system preference for overriding Email
SMS send driver from address.

How to test:
1) Run tests in t/db_dependent/Letters.t
2) All tests should pass

Sponsored-by: Gothenburg University Library

Signed-off-by: Michal Denar <black23@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-13 14:36:55 +00:00
8205305b4f Bug 21804: Remove C4::Accounts::getcharges again
Test plan:
1) git grep getcharges
  -> should return no occurences
2) prove t/db_dependent/Accounts.t
  -> should pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
2018-11-12 16:28:22 -03:00
04d137b06d Bug 19349: (QA follow-up) Cosmetic changes
[1] Construction with a // b instead of a; unless( a ) b;
[2] Error checking on subfieldCode
[3] Add explanation how to fill preference

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 20:53:28 +00:00
Julian Maurice
42e9c49934 Bug 19349: Store record's creator and last modifier in record
Test plan:
1. Run updatedatabase.pl
2. Set sysprefs MarcFieldForCreatorId, MarcFieldForCreatorName,
   MarcFieldForModifierId, MarcFieldForModifierName
3. Create a new biblio
4. Verify that the fields are correctly filled
5. Logout and login as another user
6. Modify the same biblio
7. Verify that only the fields for last modifier have been modified

Works perfectly.

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 20:53:27 +00:00
7d10549ae8 Bug 21205: Replace C4::Items::GetOrderFromItemnumber calls
This is done to ease the move of C4::Items (bug 18252) to Koha::Items

  my @itemnumbers = GetItemnumbersFromOrder($order->{ordernumber});
will become
  my @itemnumbers = $order_object->items->get_column('itemnumbers');

Test plan:
- Create an order with several items
- Receive some items

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 20:47:16 +00:00
Matthias Meusburger
9dc5a53daa Bug 18507: Shibboleth auto-provisioning - Sync
This patch allows to update borrowers informations with Shibboleth attributes
upon login.

Test plan:
1. In $KOHA_CONF, check that //shibboleth/sync is set to 1
2. Find an existing user and change one of the values mapped with a Shibboleth attribute
3. Log in using Shibboleth
4. Check that the value has been updated with the Shibboleth attribute.

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 20:43:32 +00:00
Julian Maurice
d708255c7a Bug 15395: Allow correct handling of plural translation
Locale::Maketext does not allow correct handling of plural translation
for languages that have more than one plural forms.
Locale::Messages does.

So Koha::I18N is now a wrapper around Locale::Messages, just like
Locale::TextDomain, and export the same symbols as Locale::TextDomain.
You can refer to documentation of Locale::TextDomain to know how to use
exported subroutines.

Example usage:
  __("Hi")
  __x("Hi {name}", name => 'Bob')
  __n("item", "items", $num_items)
  __nx("one item", "{count} items", $num_items, count => $num_items)
  __p("Bibliographic record", "item")

This patch also brings Koha::I18N power to Template::Toolkit templates
by adding a TT plugin.
This plugin can be used like this:

[%# USE the plugin and define some macros %]
[% PROCESS 'i18n.inc' %]

[%# tn is the equivalent of __n %]
[%# macro names can't start with underscore, t is for "translate" %]
[% tn('item', 'items', num_items) %]
Extraction of strings from templates is a bit complicated and use
Template::Parser and PPI. Template is compiled into Perl code and then
analyzed by PPI. It is slow, but should be correct even with complex
constructions.

Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon
Add dependency to Locale::Messages and PPI

Test plan for translation in Perl code:
1. Open a .pl script or .pm module with your favorite text editor
2. Add 'use Koha::I18N;' in the beginning of file
3. Use one of the subroutines exported by Koha::I18N and be sure to have
   a way to visualize the result (pass result to the template for
   example, or simply warn and watch the log file)
4. cd misc/translator && ./translate update fr-FR # try other languages
5. Open misc/translator/po/fr-FR-messages.po and translate your
   string(s)
   You may need to change the "Plural-Forms" header. See
   https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html
6. ./translate install fr-FR
7. Use your web browser to go to the page that should display the
   translation, change language and verify the translation is correct
8. prove t/Koha/I18N.t

Test plan for translation in templates:
1. Open a template file (.tt or .inc) with your favorite text editor
2. Add the PROCESS directive mentioned above in the beginning of file
3. Use one of the t* macros defined in i18n.inc. They are used like
   their "__" equivalent, with one difference: the 'x' variants take a
   hashref instead of a hash as last parameter
4. cd misc/translator && ./translate update fr-FR
5. Open misc/translator/po/fr-FR-messages.po and translate your
   string(s)
6. ./translate install fr-FR
7. Use your web browser to go to the page that should display the
   translation, change language and verify the translation is
   correct

Signed-off-by: Marc Véron <veron@veron.ch>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 15:55:50 +00:00
1c58a309f1 Bug 15184: Add the ordernumbers filters to GetHistory
It adds the ability to return the result of GetHistory given a list a
ordernumbers

Sponsored-by: BULAC - http://www.bulac.fr/

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

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 15:52:57 +00:00
87b877e492 Bug 21413: Fix condition to avoid operator bnding isssues
The original WHERE condition wasn't enclosed in parenthesis, and hence
was subject to binding precedence issues in some situations.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 13:32:42 +00:00
a74f8eb634 Bug 21413: Inventory - Skip items with waiting holds
This patch enables items to be skipped for inventory stocktaking
if the item is currently a waiting hold. The intention is to skip
items that should be on the holds waiting shelf in the library.

Test Plan:
1) Apply this patch
2) Generate a list of barcodes ordered by callnumber
3) Select one of those items and place it on hold, then confirm the hold
4) Remove that barcode from the list of barcodes
5) Browse to the inventory tool, choose your barcodes file
6) Run the inventory tool with that barcode file.
7) Note the tool says that item should have been scanned
8) Click the browsers back button to return to the previous page
9) Check the checkbox for "Skip copies marked as waiting holds"
10) Run the tool again, not it does not flag that item as previously

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 13:32:41 +00:00
Joonas Kylmälä
83e0765f44 Bug 21015: fix performance issue with C4::Members
loading Koha::Schema ("use Koha::Schema;") takes significantly time as
it loads almost a couple hundred classes. Koha::Database has done that
already once and we can use it to get the ResultSet "Borrower" as
well, so let's use that. This also make the code more unified because
Koha::Database is used throughout the code instead of Koha::Schema.

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 02:18:48 +00:00
543630b3c4 Bug 21719: Fix typos
This patch was generated using codespell

Test plan:
Read through changes and confirm they make sense

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

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

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-08 02:18:46 +00:00
4ed7a533ab Bug 15494: (follow-up) Move yaml syspref code to its own sub in SySpref object
To test:
1 - prove -v t/db_dependent/Koha/Config/SysPrefs.t

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-07 21:39:39 +00:00
114c1dfb2c Bug 15494: (QA follow-up) Additional polishing
[1] Fix two typos in Circulation.t.
    Although the test does not fail, line 2127 contains two typos.
    Changing INVISILE to INVISIBLE :)
    And type should be itype.
[2] Remove $yaml as leftover from older code.
[3] Add a next when the split on /:/ does not give two results. This will
    prevent uninit warnings (although still disabled now in Circulation).
[4] For the same reason we should switch the lines for NULL and empty
    string. The undefs you insert should trigger a warn.
[5] The line for empty string should not insert undef, but empty string.
    For the same reason adding the condition defined($_) ...
    And proving it by adding two tests for the opposite values of
    callnumber and itemnotes.
[6] Adding a strip spaces around the fieldname. User friendly..

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-07 21:39:39 +00:00
72750cf3b7 Bug 15494: Update display of renewal errors
To test:
1 - Define a rule to deny renewal
2 - Checkout an item meeting this rule
3 - Attempt renew an item via the OPAC
4 - Attempt renewal via SIP
5 - Attempt renewal via staff client
6 - Verify errors are reasonable

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

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-07 21:39:38 +00:00
633a587213 Bug 15494: Add _items_denied_renewal sub to C4:Circulation::CanBookBeRenewed
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-07 21:39:38 +00:00
fb15a96310 Bug 20629: Remove ability to 'reverse' payments
Test Plan:
1) Apply this patch
2) Note all references to reversing payments have been removed
3) Note ability to void payments remains unchanged

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-06 16:32:34 +00:00
4478545368 Bug 20078: Add accelerated reading indexes to getIndexes
To test:
1 - Find a bib with 526$c and 526$d populated, or populate them
 e.g. 526$a 5.5 | 526$d 14
2 - Try searching for 'arl:5.5' or 'arp:14'
3 - Your record should be returned in results
4 - Add the indexes as dropdowns by entering the following in intranetuserjs:
$(document).ready(function(){
        // Add Accelerated Reading Level to advanced search
    if (window.location.href.indexOf("catalogue/search.pl") > -1) {
        $(".advsearch").append('<option value="arl">Accelerated Reading Level</option>');
    }

    // Add Accelerated Reading Point to advanced search
    if (window.location.href.indexOf("catalogue/search.pl") > -1) {
        $(".advsearch").append('<option value="arp">Accelerated Reading Point</option>');
    }
});
5 - Search using these in the dropdown, your record will not be found
6 - Apply patch
7 - Repeat searches directly and by using dropdown, your record should be returned.
8 - prove t/db_dependent/Search.t

Signed-off-by: Margie Sheppard - Central Kansas Library System CKLS <msheppard@ckls.org>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-06 12:22:47 +00:00
f683c181ac Bug 21650: Remove the subroutine C4::Items::GetLastAcquisitions
C4::Items::GetLastAcquisitions has been added by
  commit 7753bbad4f
  Adding Some new functions

(?)

Apparently it has never been used, we should remove it to avoid unnecessary maintenance.

Test plan:
  git grep GetLastAcquisitions
must not return any results

NOTE: POD Coverage test failure is expected on removal

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-05 14:41:11 +00:00
7a1641cd41 Bug 14385: DBRev 18.06.00.050
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:34:02 +00:00
70651422a7 Bug 14385: (QA follow-up) Additional changes and fixes
[1] searchResults: second my $interface can be removed: unused
[2] call of getitemtypeimagelocation on L2119 needs interface key
[3] ISBDdetail: No need to find patron again (line 182 vs 84)
[4] opac-search: No need to find patron twice (657 and 631)
[5] tabs on line 2220 of C4/Search.pm (qa tools warn)
[6] Ugly hack to overcome "Undefined subroutine &C4::Items::ModZebra"
    by loading C4::Items before C4::Biblio when running tests
    Koha/BiblioUtils/Iterator.t and Labels/t_Label.t.
    This is a more general problem that needs attention somewhere else.
    It seems that Biblio.pm is one of the suspects.
[7] This patch set makes Search.t crash/fail with me. Note that without
    these patches Search.t still passed! Why o why..
    A little debugging pointed me to a missing MPL branch (aarg).
    Adding the simple test on the result of Libraries->find in
    C4::Biblio::GetAuthorisedValueDesc made the test continue.
[8] Resolve: Variable "$borcat" is not available at opac-detail.pl line 246
    Lexical $borcat cannot be used in sub searchAgain in opac-detail.pl
    under Plack. Must be defined with our (or passed as argument).
[9] Resolve crash on TWO serious typos in opac-basket on ONE line:
        Koha::Patron->find({ borrowernumber -> $borrowernumber })
    Yeah: find is in Koha::Patrons and we need => !!
    No need to pass a hash to find method btw for a pk value.
[10] Serious bugfixing here. Add List::Util to opac-basket.
    Can't locate object method "none" via package "1".
    You can't test everything :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
After this longer list I renamed Final to Additional in the patch title :)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:12 +00:00
Mark Tompsett
e1b5fa657d Bug 14385: Squash of a lot of patches rebased
- Added missing GetHiddenItems parameter change case
     Without this prove t had a failure.

- Always use mocks, not set_preference

- Tweaks so t/db_dependent/00-strict.t passes
     There was a typo botcat vs borcat and borrowernumber was never
     defined. Grabbing from userenv, like other code does.

- Tweak t/db_dependent/Items.t to fully test changes
     This will test all the if structures fully in GetHiddenItemnumbers.
         prove t/db_dependent/Items.t

- Tweak borrower category code
     $borrower->{categorycode} on a Koha::Patron is not the
     same as $borrower->categorycode. Fixed error.

- Search was returning URLS for wrong interface
     There was one search context place wrong. Changed it to $is_opac
     as the logic for setting $is_opac was modified correctly.

- Corrected issues with category code.
     When a user isn't logged in, $borrower is undef and causes error
     when determining category code. Added conditional check.

- Properly trigger all changes in C4/Search.pm

- Fix QA Test tool failures
     C4/Search.pm had some tabs.

- Add some commenting to make sense of logic

- Refactor EmbedItemsInMarcBiblio parameters to hashref

- Trigger GetMarcBiblio's EmbedItemsInMarcBiblio call.
     prove t/db_dependent/Items.t

- Add missing test to trigger Koha/BiblioUtils/Iterator change

- Add borrower category overrides
     These files generally add borcat parameter to GetMarcBiblio.
     Others might include correction of filtering of items
     (opac-basket), or a comment as to why no changes were done
     (opac-search).

     In the case of opac-search, correcting the first FIXME will
     likely correct the OpacHiddenItems issues on tags. As such,
     that is beyond this bugs scope.

     Some code had loop optimizations and fixes made, like a
     'next unless $record' when the biblio shouldn't even be in
     the list.

- Modify opac-ISBDdetail and opac-MARCdetail
     Both files had similar logic. They were rearranged and
     optimized, so that both files would have practically identical
     initial blocks of code.

     Optimizations were possible, because GetMarcBiblio
     returns a filtered record, so that there is no double call
     (once in the opac-### file and once in GetMarcBiblio) to
     GetHiddenItemnumbers.

- Fix hiding in opac-tags
     opac/opac-tags.pl was not properly hiding.

     There is currently one known bug associated with tags left.
     If you have two biblios tagged by different people with the
     same tag, the opac-search will show the one you tagged that
     is supposed to be hidden, because tag searches work differently
     than regular searches. This is beyond the scope of this bug.
     See the FIXME's in opac/opac-search.pl

- Trigger the C4::ILSDI::Services changes
     prove t/db_dependent/ILSDI_Services.t

- Added missing 'my'

- Test C4/Labels/Label.pm changes

- Improve C4::Record::marcrecord2csv test cases

- Corrected opac-details searchResult call

- Fix breaking issues constraint in ITerator test

- Fix ILSDI_Services test when clubs with branch exist

- Rebased again!
- Rebased t/db_dependent/Items.t conflict.

The test plan is in comment #112 last I checked.

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:12 +00:00
Chris Cormack
5e4e10c4ca Bug 14385: Extend OpacHiddenItems to allow specifying exempt borrower categories
Edit: Fixing merge conflicts in
 - t/db_dependent/Items.t
 - t/db_dependent/Search.t
 - C4/Search.pm

Changes the API for calling GetHiddenItems and all the places in the code that call it. This is to allow borrower categories to be passed in.
Adds an OpacHiddenItemsExceptions syspref to allow certain borrower categories to be able to see items, even if they are marked hidden by OpacHiddenItems

To test:

1) Make two borrowers, one in a category that should see everything (ie Adult), and another in a category that should only see certain things (ie Adult - exceptions)
2) Add the borrower that can see everything (the Adult) to OpacHiddenItemsExceptions
3) To the OpacHiddenItems syspref, add an item type (ensure that you have some records that fall under this type in your library).
4) Log in as the borrower that should only see certain things (Adult - exception)
5) Do a search, filtered to show records which are the item type that you specified in the OpacHiddenItems syspref. No records should show for this borrower as this item type is hidden to them.
6) Log in as the borrower that should see everything (Adult)
7) Do the same search. There should be results from this search, as this borrower category has been specified as an exception to the hidden items

Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>

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

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:09 +00:00
32c4dbfd68 Bug 21606: [sql_modes] Fix matching rules insert
Fix:
Incorrect integer value: '' for column 'offset'
Incorrect integer value: '' for column 'score'

Test plan:
Add/edit matching rules

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:08 +00:00
7085772e8d Bug 21604: Fix add/edit fund/budget
[Strict SQL modes context]

For insert aqbudgetperiods:
Incorrect integer value: '' for column 'budget_period_id'

For insert/update aqbudgets
Incorrect integer value: '' for column 'budget_owner_id'
Incorrect decimal value: '' for column 'budget_encumb'

Test plan:
Add/edit budgets and funds

A good example of why we should use Koha::Object for all our objects

Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-02 10:33:01 +00:00
515ed462cf Bug 21556: Do not crash if a biblio is deleted twice
To recreate:
- Go to a bibliographic detail page
- Delete it
- Go back
- Delete it again
=> Without this patch you will get a 500
 Can't call method "holds" on an undefined value at
 /home/vagrant/kohaclone/C4/Biblio.pm line 406.
=> With this patch applied it will silently redirect you to the search
form.

Note: We could/should improve the behavior and display a message, but
DelBiblio will need to be moved to Koha::Biblio->delete and other
callers adjusted

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-01 19:05:12 +00:00
b5c456479d Bug 21725: Use full statement in HAVING rather than constructed column
To test:
1 - prove -v t/db_dpendent/Acquisition/GetBasketsInfosByBookseller.t
2 - Apply patch
3 - prove -v t/db_dpendent/Acquisition/GetBasketsInfosByBookseller.t
4 - It passes
5 - Have some open/closed and some fully and not full received baskets
6 - View a vendor and confirm baskets are shwn as expected
7 - Check 'Show all baskets' shows all baskets

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Note sure this is the best thing but it fixes the issue

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-01 19:04:30 +00:00
8ceba54c8d Bug 21742: Fix YouTube variable propagation
The loop to find all media has an $isyoutube variable. This was declared
outside the loop and never reset. Once a YouTube video was encountered
all subsequent videos were considered as YouTube. This patch reduces the
scope of the variable and resets per loop.

To test:
1 - Enable HTML5Media and HTML5MediaYouTube
2 - Add 856$u to record for a youtube vido and a pdf (sample record
        attached to bug)
3 - View the record, the Play media tab has two video boxes
4 - Play the second, it fails
5 - Apply patch
6 - View record, Play media should include only one video link

Signed-off-by: Devinim <kohadevinim@devinim.com.tr>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-01 18:59:33 +00:00
8f6bd94150 Bug 21681: Improve the account lines fetch
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-31 13:52:08 +00:00
655caa61e6 Bug 21681: Remove C4::Accounts::getcharges
Test plan:
0) Do not apply the patch
1) Enable ILS-DI
2) Go to
<opac_url>/cgi-bin/koha/ilsdi.pl?service=GetPatronInfo&patron_id=51&show_fines=1
use patron with some paid and unpaid fines
3) Save the page for later usage
4) Apply the patch
5) Repeat 2 with same patron and compare the saved result with the new
one, they should be same
6) git grep getcharges should return no occurance
7) prove t/db_dependent/Accounts.t

Co-authored-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-31 13:52:07 +00:00
77ea468639 Bug 18720: Use exception instead of die in GetBasketAsCsv
Test plan:
prove t/db_dependent/Acquisition/GetBasketAsCSV.t

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-31 13:47:17 +00:00
541a90765c Bug 21581: (QA follow-up) Update comment according to code in SearchAuthorities
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-31 13:17:48 +00:00
6093e08443 Bug 21581: Pass through authorities indexes
To test:
 1 - Define a rule:
  Matching rule code: LCAUTH
  Match threshold: 100
  Record type: Authority record
  MATCH POINTS
  Search index: LC-card-number
  Score: 100
  Tag: 010
  Subfield: a
 2 - Have a record with an LC number in 010
 3 - Export it
 4 - Edit the record in Koha, move the value from 010 to another field
 5 - Import the original record
 6 - Use the matching rule above, you expect it won't match (nothing in 010 anymore)
 7 - It matches?!?!?!
 8 - Enable zebra logging in koha-conf.xml
  <zebra_loglevels>none,fatal,warn,request,info</zebra_loglevels>
 9 - tail -f /var/log/koha/kohadev/zebra-output.log
10 - On manage marc apply matchign with no rule, then reapply with rule
11 - View the output in the log - the index searched is 'Any'
12 - Apply patch
13 - Reapply matching
14 - Note we search the expected index and no match is found

NOTE: By applying bug 21579, I was able to view the difference, and
      see that it didn't match identically, because I had moved the
      value from 010 to 155. No log checking needed. clean/delete batch.
      Apply patch, no match. clean/delete batch.
      Edit authority back, matches.

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-31 13:17:47 +00:00
7dd4d60c8a Bug 13098: Make _FixAccountForLostAndReturned refund the right thing
This patch changes the logic inside the method, to make it match the
behaviour described on the tests.

It uses the existing offsets on the account_offsets table to gather
information about the right things to refund.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Circulation.t
=> FAIL: Tests don't pass!
- Apply this patch
- Run
 k$ prove t/db_dependent/Circulation.t
=> SUCCESS: Tests pass!
- Sign off :-D

Followed test plan, patch works as described. All three patches pass QA
test tool
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-31 13:13:37 +00:00
Andrew Isherwood
4858c4f736 Bug 21641: (follow-up) Address wrong call method
This patch substitutes C4::Accounts->getnextacctno for
C4::Accounts::getnextacctno since getnextacctno is only expecting to be
passed a borrowernumber

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-29 00:22:17 +00:00
Andrew Isherwood
6fba062d29 Bug 21641: (follow-up) Address additional call
This patch makes the same change as the main patch, just for an
additional occurance of getnextacctno

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-29 00:22:17 +00:00
344dba2fb8 Bug 21641: Added C4::Accounts to getnextacctno() call
Test plan:
1. Fetch and checkout the master branch
2. Checkout an item with a rental charge associated with it to a user
and notice an error is thrown
3. Apply patch
4. Restart memcached and plack
5. Repeat steps 1 and 2 and notice the checkout happens successfully

Sponsored-By: Catalyst IT

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-29 00:22:16 +00:00
89bad1fa46 Bug 21680: Remove dead code - C4::Accounts::fixaccounts
Test plan:

git grep fixacccounts
-- without patch there are 3 occurences
-- with patch - no occurence of fixaccounts

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-26 17:09:59 +00:00
51ce2ebfe1 Bug 21654: Remove refund_lost_item_fee_rules.sql from the installer process
This is a follow-up patch for bug 14048 comment 97

Test plan:
  git grep refund_lost_item_fee_rules.sql
should not return any results

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-26 17:09:59 +00:00
41397c8553 Bug 21603: Remove incorrect GROUP BY from C4::CourseReserves
'koha_kohadev.c.department' isn't in GROUP BY

Test plan:
Prove that the test fail without this patch and pass with this patch
applied (switch on the SQL modes)

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-26 16:33:14 +00:00
85da34c94a Bug 15766: Fix incorrect GROUP BY
'koha_kohadev.creator_batches.description' isn't in GROUP BY

Fix t/db_dependent/Creators/Lib.t if strict sql modes is on

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-24 17:15:36 +00:00
Aleisha Amohia
ba8ec9bfe2 Bug 15766: (follow-up) Hide description field if the batch is empty
Because if it's empty, it hasn't actually been created yet. Hiding the
field prevents a lot of errors

Sponsored-by: Catalyst IT

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-24 13:52:42 +00:00
Aleisha Amohia
5a4f7c6c0e Bug 15766: Adding descriptions to patron card batches
This patch adds a 'description' column to the creator_batches table. The
description for a batch can be added and updated using ajax.

To test:
1) Apply patch and update database (you will have to restart memcached)
2) Go to Tools -> Patron card creator -> Manage batches
3) There should now be a Description column next to Batch ID. This
will be empty (as none of the batches have descriptions yet)
4) Click Edit for any batch
5) Notice new Batch description text field. Enter a description for
the batch in here and click Save description. Some text should show
saying the description was saved.
6) If you go back to the manage batches page, the description should
now show under the Description column.
7) Go to Tools -> Label Creator -> Manage labels
8) Repeat steps 3 to 6

Sponsored-by: Catalyst IT

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

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-24 13:52:40 +00:00
c5641af82a Bug 15486: (follow-up) Default to unlimited
While it doesn't have a practical effect (the variable is reset several
lines below) I agree this should be explicitly set to its default.

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

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-24 13:24:06 +00:00
fb0ab55fd0 Bug 15486: Extend CanItemBeReserved so it handles daily holds limits
This patch implements the required changes in
C4::Reserves::CanItemBeReserved so it implements a daily limit on holds.

It returns the 'tooManyReservesToday' string if the policy doesn't allow
placing the hold. It returns 'OK' (current behaviour) otherwise.

To test:
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/Holds.t
=> FAIL: Tests fail because the error condition is not making
CanItemBeReserved return the desired error code.
- Apply this patch
- Run:
 k$ prove t/db_dependent/Holds.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-24 13:24:05 +00:00