koha.git
7 years agoBug 15879: Allow multiple plugin directories to be defined in koha-conf.xml
Kyle M Hall [Mon, 22 Feb 2016 14:25:18 +0000 (14:25 +0000)]
Bug 15879: Allow multiple plugin directories to be defined in koha-conf.xml

It would be very useful to be able to define multiple plugin directories
in the Koha conf file. This would allow for ease of plugin development
so that each plugin installed can live in its own git repository. For
compatibility, the first plugindir instance defined should be the one
used for uploading plugins via the web interface.

Test Plan:
1) Apply this patch
2) Define a second pluginsdir line in your koha-conf.xml
3) Clone the kitchen sink plugin to this new path like this:
   git clone https://github.com/bywatersolutions/koha-plugin-kitchen-sink.git /path/to/new/plugins/dir
4) Restart memcached if you are running it
5) The Kitchen Sink plugin should now appear in your list of plugins!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I rebased it against master and tested it on a kohadevbox

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17234: Test the column and constraint non-existence
Jonathan Druart [Mon, 2 Jan 2017 10:36:53 +0000 (11:36 +0100)]
Bug 17234: Test the column and constraint non-existence

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17234: Two new functions lack tests
Mark Tompsett [Fri, 30 Dec 2016 19:08:33 +0000 (14:08 -0500)]
Bug 17234: Two new functions lack tests

This adds two tests to t/db_dependent/Installer.t

TEST PLAN
---------
1) Apply patch
2) prove -v t/db_dependent/Installer.t
   -- column and constraint tests were added.
3) run koha qa test tools

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17234: Move new subroutines to C4::Installer
Jonathan Druart [Tue, 27 Sep 2016 07:28:09 +0000 (08:28 +0100)]
Bug 17234: Move new subroutines to C4::Installer

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17234: Add constraint_exists and column_exists to updatedatabase.pl
Jonathan Druart [Tue, 20 Sep 2016 15:55:40 +0000 (16:55 +0100)]
Bug 17234: Add constraint_exists and column_exists to updatedatabase.pl

These 2 subroutines will help us deal with the absense of ALTER IGNORE
TABLE

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17234: Follow up to handle new problems
Mark Tompsett [Mon, 19 Sep 2016 18:27:14 +0000 (14:27 -0400)]
Bug 17234: Follow up to handle new problems

Bug 16276 added two more ALTER IGNORES. This removes them.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17234 - updatedatabase.pl's ALTER IGNORE break with mysql 5.7.4+
Blou [Fri, 2 Sep 2016 12:05:48 +0000 (08:05 -0400)]
Bug 17234 - updatedatabase.pl's ALTER IGNORE break with mysql 5.7.4+

The doc says: "As of MySQL 5.7.4, the IGNORE clause for ALTER TABLE is removed and its use produces an error."
This fix replaces ALTER IGNORE with ALTER in updatedatabase.pl

To TEST, try an upgrade from 3.18 to 3.22 after installing the latest mysql (at least 5.7.4, by the doc).
Some will fail with error

DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'IGNORE TABLE aqbasket
            ADD KEY authorisedby (authorisedby)' at line 1 [for Statement "
        ALTER IGNORE TABLE aqbasket
            ADD KEY authorisedby (authorisedby)
    "] at ./installer/data/mysql/updatedatabase.pl line 10563.

0) Find a database on 3.18, save it.
1) Set your code base to 3.22(or master)
2) run updatedatabase.pl
3) See the errors.
4) Apply the patch
5) Reload the 3.18 database
6) succeed with updatedatabase.pl

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15897: Update ->pay POD
Jonathan Druart [Tue, 10 Jan 2017 09:21:23 +0000 (10:21 +0100)]
Bug 15897: Update ->pay POD

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15897 [QA Followup] - Update unit tests
Kyle M Hall [Mon, 9 Jan 2017 17:19:54 +0000 (17:19 +0000)]
Bug 15897 [QA Followup] - Update unit tests

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15897 - Folowup Revert "Bug 15896: [QA Follow-up] Add accountlines_id parameter...
Josef Moravec [Wed, 7 Dec 2016 02:41:08 +0000 (02:41 +0000)]
Bug 15897 - Folowup Revert "Bug 15896: [QA Follow-up] Add accountlines_id parameter in paycollect"

This reverts commit b6d5748c001febc5acd67938d12ba25844d11fbc.

As this bug report no more uses the accounline_id parameter to identify
account lines to pay in Koha::Account->pay, it should revert this, to
use the new notation everywhere.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15897 - Use Koha::Account::pay internally for recordpayment_selectaccts
Kyle M Hall [Wed, 24 Feb 2016 13:30:07 +0000 (13:30 +0000)]
Bug 15897 - Use Koha::Account::pay internally for recordpayment_selectaccts

This is the third patch in a series to unify all payment functions into
a single mathod

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Accounts.t
3) Test fine payment via the "Pay selected" button

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>
7 years agoBug 14610 [QA Followup] - Minify opac.css
Kyle M Hall [Tue, 3 Jan 2017 11:01:08 +0000 (11:01 +0000)]
Bug 14610 [QA Followup] - Minify opac.css

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17830: CSRF - Handle unicode characters in userid
Jonathan Druart [Thu, 29 Dec 2016 16:54:40 +0000 (17:54 +0100)]
Bug 17830: CSRF - Handle unicode characters in userid

If the userid of the logged in user contains unicode characters, the token
will not be generated correctly and Koha will crash with:
  Wide character in subroutine entry at /usr/share/perl5/Digest/HMAC.pm line 63.

Test plan:
- Edit a superlibrarian user and set his/her userid to '❤' or any other strings
with unicode characters.
- Login using this patron
- Search for patrons and click on a result.

=> Without this patch, you will get a software error (with "Wide
character in subroutine entry" in the logs).
=> With this patch, everything will go fine

You can also test the other files modified by this patch.

Signed-off-by: Karam Qubsi <karamqubsi@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17569: [QA Follow-up] Small changes
Marcel de Rooy [Fri, 2 Dec 2016 09:30:55 +0000 (10:30 +0100)]
Bug 17569: [QA Follow-up] Small changes

Patron.pm: Adds two missing semicolons at the last statement. Not strictly
needed, but strongly recommended.
Patrons.t: Add a test description, remove two comments that refer to
previously hardcoded dates.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17569: Do not limit by branch if option is not passed
Jonathan Druart [Tue, 8 Nov 2016 08:07:02 +0000 (08:07 +0000)]
Bug 17569: Do not limit by branch if option is not passed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17569: Remove C4::Members::GetUpcomingMembershipExpires
Jonathan Druart [Tue, 8 Nov 2016 08:02:38 +0000 (08:02 +0000)]
Bug 17569: Remove C4::Members::GetUpcomingMembershipExpires

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membe...
Jonathan Druart [Mon, 7 Nov 2016 14:38:17 +0000 (14:38 +0000)]
Bug 17569: Koha::Patrons - Move GetUpcomingMembershipExpires to search_upcoming_membership_expires

This patchset moves the C4::Members::GetUpcomingMembershipExpires
subroutine code to the Koha::Patrons->search_upcoming_membership_expires
method.
This subroutine was used from only 1 place, so it's an easier to move.

Test plan:
Use the membership_expiry.pl cronjob script using the different
options.
The behavior should be the same as prior to this patch.

  prove t/db_dependent/Koha/Patrons.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17569: Move tests to the patron module test file
Jonathan Druart [Mon, 7 Nov 2016 14:34:32 +0000 (15:34 +0100)]
Bug 17569: Move tests to the patron module test file

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17569: Rewrite existing tests to make them reusable and more robust
Jonathan Druart [Mon, 7 Nov 2016 13:18:54 +0000 (13:18 +0000)]
Bug 17569: Rewrite existing tests to make them reusable and more robust

In order not to cheat you, I am rewriting the tests in a separate
commit.
You can confirm that they pass with the other patches.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 13726 - (QA followup) Fix vendor retrieval in invoices.pl
Nick Clemens [Wed, 28 Dec 2016 15:10:26 +0000 (15:10 +0000)]
Bug 13726 - (QA followup) Fix vendor retrieval in invoices.pl

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 13726: Fix for serials/acqui-search-result.pl
Jonathan Druart [Tue, 29 Nov 2016 09:16:55 +0000 (09:16 +0000)]
Bug 13726: Fix for serials/acqui-search-result.pl

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>
7 years agoBug 13726: Make Koha::Acq::Bookseller using Koha::Object
Jonathan Druart [Tue, 17 Feb 2015 13:37:10 +0000 (14:37 +0100)]
Bug 13726: Make Koha::Acq::Bookseller using Koha::Object

This patch create a Koha::Acquisition::Booksellers module and
Koha::Acquisition::Bookseller::Contract[s] modules.

All code in the acquisition module is adapted to use the CRUD methods of
Koha::Object[s].
The former C4 routines are removed.

Test plan:
Since a lot of files are impacted by this patch, try a complete
acquisition workflow and try to catch errors.
Be focused on bookseller and bookseller' contacts data.

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>
7 years agoBug 17820: use ->find instead of search->next
Jonathan Druart [Wed, 28 Dec 2016 12:14:06 +0000 (13:14 +0100)]
Bug 17820: use ->find instead of search->next

From C4::Auth:
  my $patron = Koha::Patrons->search({ userid => $userid })->next;

This should be replaced with
  my $patron = Koha::Patrons->find({ userid => $userid });

userid is a unique key

Caught with NYTProf:
 # spent 78.9ms making 1 call to Koha::Objects::next

Test plan:
Login at the intranet
Reload the page
=> You must still be logged in

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested by enabling TrackLastPatronActivity and logging in again.
Verified lastseen column in borrowers.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17767 - DBRev 16.12.00.001
Kyle M Hall [Wed, 28 Dec 2016 13:56:07 +0000 (13:56 +0000)]
Bug 17767 - DBRev 16.12.00.001

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17767: (followup) Rename test file
Tomas Cohen Arazi [Wed, 14 Dec 2016 15:06:59 +0000 (12:06 -0300)]
Bug 17767: (followup) Rename test file

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17767: Unit tests
Tomas Cohen Arazi [Thu, 15 Dec 2016 04:31:01 +0000 (01:31 -0300)]
Bug 17767: Unit tests

This patch introduces tests for the new behaviour, and also
enhances the existing ones aiming to reach full coverage.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17767: Make Koha::Patron::Modification handle extended attributes
Tomas Cohen Arazi [Tue, 13 Dec 2016 20:45:41 +0000 (17:45 -0300)]
Bug 17767: Make Koha::Patron::Modification handle extended attributes

This patch makes Koha::Patron::Modification aware of the new extended_attributes
column, which is expected to contain valid JSON data.

The ->store method is modified so it validates the field value (i.e. the content
is decoded using the JSON library) and raises a convenient exception in case of
failure.

This behaviour change is covered by the provided unit tests.

To test:
- Apply the patchset
- Run:
  $ prove t/db_dependent/Koha/Patron/Modifications.t
=> SUCCESS: Tests make sense, and they pass
- Sign off :-D

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17767: DBIC update
Tomas Cohen Arazi [Tue, 13 Dec 2016 20:42:53 +0000 (17:42 -0300)]
Bug 17767: DBIC update

This patch can be skipped by the RM, it just updates the DBIC schema files.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17767: Add borrower_modification.extended_attributes
Tomas Cohen Arazi [Tue, 13 Dec 2016 20:42:35 +0000 (17:42 -0300)]
Bug 17767: Add borrower_modification.extended_attributes

This patch changes the DB structure adding borrower_modifications.extended_attributes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17679: C4::Circulation - Remove unused GetItemIssues
Jonathan Druart [Fri, 25 Nov 2016 10:52:11 +0000 (11:52 +0100)]
Bug 17679: C4::Circulation - Remove unused GetItemIssues

Ready for an archaeology course?

C4::Circulation::GetItemIssues is only used once, from
catalogue/issuehistory.pl
This call has been added by
  commit 95d6452462a560ba0c0ac859a2cfb7783c25c925
    Adding some more information on issuehistory.
which says "Adding itemnumber to issuehistory.pl API so that one could search
for issuehistory of a specific item."
So it added the ability to see the item issue history but did not
provide a way to access it via the interface.
It's ok so far but this subroutine is broken since
  commit aa114f53499b9cffde0571fe7e08622f9c9a332a
    Bug 5549 : Only use DateTime for issues table
because of this change:
-    my $today = C4::Dates->today('iso');
+    my $today = DateTime->now( time_zome => C4::Context->tz);

I let you catch the typo ;)
And since this commit the subroutine explodes with "The following
parameter was passed in the call to DateTime::from_epoch but was not
listed in the validation options: time_zome"

Since it has never been raised by someone and that the feature is
hidden, I'd recommend to simply remove it.

Note that the "Checked out from" column would have been wrong even if we
fixed all the previous issue.

Test plan:
Just dig into the code and confirm what this commit message tells

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Looks fine for me.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17584 [QA Followup] - Rename Koha::Patron::get_checkouts to Koha::Patron::checkouts
Kyle M Hall [Fri, 23 Dec 2016 14:07:42 +0000 (14:07 +0000)]
Bug 17584 [QA Followup] - Rename Koha::Patron::get_checkouts to Koha::Patron::checkouts

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17584: Add the Koha::Patron->get_checkouts method
Jonathan Druart [Tue, 8 Nov 2016 12:52:52 +0000 (12:52 +0000)]
Bug 17584: Add the Koha::Patron->get_checkouts method

Test plan:
  prove t/db_dependent/Koha/Patrons.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17209: Remove use of onclick from masthead
Aleisha Amohia [Mon, 29 Aug 2016 04:47:36 +0000 (04:47 +0000)]
Bug 17209: Remove use of onclick from masthead

There are two instances which use onclick in the OPAC masthead: clearing
search history, and logging out.

To test:
Confirm that clearing search history using the 'x' in the masthead, and
logging out by clicking 'Log out' in the masthead, work the same before
and after the patch.

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16072: Changing all instances of 'loading-small.gif' to 'spinner-small.gif' and...
Aleisha Amohia [Mon, 12 Dec 2016 00:00:30 +0000 (00:00 +0000)]
Bug 16072: Changing all instances of 'loading-small.gif' to 'spinner-small.gif' and removing loading-small.gif file.

Confirm that I have not missed any places where there is 'loading-small.gif'
Have amended patch to not include OPAC changes from previous patch.

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15711: Follow up batch_id not used
Mark Tompsett [Fri, 2 Dec 2016 19:37:07 +0000 (19:37 +0000)]
Bug 15711: Follow up batch_id not used

Changed batch_id to image_id in the template, as the multiparam
would catch the array case.

TEST PLAN
---------
1) attempt to delete a selected item, but get a warning.
2) apply this follow up
3) attempt to delete a selected item, it deletes.
4) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
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>
7 years agoBug 15711: Fixing the 'Delete selected' button on patroncard images
Aleisha Amohia [Wed, 24 Aug 2016 01:08:11 +0000 (01:08 +0000)]
Bug 15711: Fixing the 'Delete selected' button on patroncard images

To test:
1) Go to Tools -> Patron Card Creator -> Manage images
2) Upload an image if you haven't already
3) Click Delete selected without selecting any images
4) Notice broken behaviour as described in Description
5) Apply patch, refresh page
6) Click Delete selected without selecting any images
7) Notice alert. Click OK
8) Select one image or more, click Delete selected
9) Notice confirm delete message.

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
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>
7 years agoBug 17785: Fix OAI-PMH's XSLT-generated URLs under Plack
Marcel de Rooy [Tue, 20 Dec 2016 15:11:36 +0000 (16:11 +0100)]
Bug 17785: Fix OAI-PMH's XSLT-generated URLs under Plack

Look at e.g. the URL for Show More at the end of the output of Records or
Identifiers. If you use Plack, you will see that it refers to
yourserver:/opac/oai.pl, which is not correct.

This is caused by using CGI's self_url in combination with script alias,
mounting point, etc. Note that we cannot solve this problem in the code of
Koha only. Since HTTP::OAI modules also call self_url, we still end up
with some wrong url's.

Instead of a larger architectural operation on Apache and Plack config files,
this patch adjusts the final xslt transformation on the OAI response.
It hardcodes the correct path only once, in a xslt variable. And replaces
all oai:OAI-PMH/oai:request/text() calls, containing wrong URLs, by this
variable.

Test plan:
Run oai.pl. Try the various verbs.
Verify that the URLs point to /cgi-bin/koha/oai.pl.

Edit: changed commit subject

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected. Good workaround until a definitive solution is implemented.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17781 - Improper branchcode set during renewal
Kyle M Hall [Thu, 22 Dec 2016 14:00:37 +0000 (14:00 +0000)]
Bug 17781 - Improper branchcode set during renewal

For no discernable reason, when AddIssue calls AddRenewal, it passes the
branchcode generated from _GetCircControlBranch. Assume
_GetCircControlBranch is set to return items.homebranch. So:
1) If an item owned by LibraryA is checked out at LibraryB, the
  statistic line branchcode will be LibraryB
2) If an item is renewed via the ajax datatables renewal function, the
  statistic line branchcode will be LibraryB the
3) If an item is renewed via scanning the item into the checkout again,
  statistic line branchcode will be *LibraryA*

This is clearly improper behavior. The renewal is taking place at
LibraryB, so the branchcode passed to AddRenewal should be LibraryB,
the logged in library. This also jives with the documentation for
the subroutine.

Test Plan:
1) Set CircControl to "the library the item is from" aka ( ItemHomeLibrary )
2) Set HomeOrHoldingBranch to 'The library the items is from" ( aka homebranch )
3) Create item with homebranch of LibraryA and holdingbranch of LibraryB
4) Set the logged in library to LibraryB
4) Check the item out to a patron at LibraryB
5) Note the statistics line has a branchcode of LibraryB
6) Check the item out again to trigger a renewal, renew the item
7) Note the statistic line has a branchcode of LibraryA!
8) Apply this patch
9) Repeat step 6
10) Note the statistics line has a branchcode of LibraryB!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Kuhn <kuhn@monterey.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17742: Use TestBuilder to create the library and patron category
Jonathan Druart [Wed, 28 Dec 2016 08:57:14 +0000 (09:57 +0100)]
Bug 17742: Use TestBuilder to create the library and patron category

Moreover we do not need to remove the existing issues and patrons

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17742: Fix t/db_dependent/Patrons.t
Josef Moravec [Tue, 6 Dec 2016 21:07:56 +0000 (21:07 +0000)]
Bug 17742: Fix t/db_dependent/Patrons.t

Moves the getting of testing date for updated_on just after the storing
the test patron data to make the gap between generating data and now
date as short as possible

Fixes test 7

Use Koha::Database instead of C4::Context->dbh

Test plan
1. prove t/db_dependent/Patrons.t
2. read the diff

NOTE: Only minor improvement could be using test builder to
      generate the category and branch codes, rather than assume
      data exists. However, that is beyond scope of this bug.

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>
7 years agoBug 17520: Do not overlap with advance_notices.pl
Jonathan Druart [Tue, 27 Dec 2016 15:57:34 +0000 (16:57 +0100)]
Bug 17520: Do not overlap with advance_notices.pl

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17520: add serialsUpdate.pl to the list of regular cron jobs
Marcel de Rooy [Mon, 31 Oct 2016 13:44:51 +0000 (14:44 +0100)]
Bug 17520: add serialsUpdate.pl to the list of regular cron jobs

This patch adds the job to debian package file and the examples file
in misc.

Test plan:
Add these lines to your cron file.
Check the results. (If an issue you expect passes the grace period defined
in the subscription, its status should go from Expected to Late.)

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>
7 years agoBug 17246: Do no support arrayref to define multiple FK
Jonathan Druart [Mon, 5 Sep 2016 08:54:35 +0000 (09:54 +0100)]
Bug 17246: Do no support arrayref to define multiple FK

Currently you can call GetPreparedLetter like:

$prepared_letter = GetPreparedLetter(
    (
        module      => 'test',
        letter_code => 'TEST_HOLD',
        tables      => {
            reserves => [ $fk1, $fk2 ],
        },
    )
);

It assumes that $fk1 is a borrowernumber and $fk2 a biblionumber.
It seems hazardous to do this guess.

I suggest to remove this feature and only allow hashref indeed.

Test plan:
Use different way to generate letters and make sure you do not reach the croak

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14637: Followup - Fix number of tests due to rebase
Kyle M Hall [Fri, 23 Dec 2016 13:35:34 +0000 (13:35 +0000)]
Bug 14637: Followup - Fix number of tests due to rebase

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14637: Fix add patron category under MySQL 5.7
Jonathan Druart [Tue, 6 Sep 2016 09:40:46 +0000 (10:40 +0100)]
Bug 14637: Fix add patron category under MySQL 5.7

If no dateofbirthrequired or upperagelimit is set on the interface, the
->store method will receive an empty string defined for these values.
For INT field, we must explicitely set these empty value to undef
instead to avoid MySQL 5.7 (and strict mode) to raise:
  DBD::mysql::st execute failed: Incorrect integer value: ' for column
'dateofbirthrequired''

Test plan:
Using MySQL 5.7 (and/or sql_mode=STRICT_TRANS_TABLES)
Create a patron category without specifying upperagelimit or
dateofbirthrequired

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14637: Fix add patron category under MySQL 5.7 - tests
Jonathan Druart [Tue, 6 Sep 2016 09:40:28 +0000 (10:40 +0100)]
Bug 14637: Fix add patron category under MySQL 5.7 - tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17783: Replace ok with is
Jonathan Druart [Tue, 20 Dec 2016 17:07:40 +0000 (18:07 +0100)]
Bug 17783: Replace ok with is

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17783: Prevent crash when providing an undefined value
Lari Taskula [Tue, 20 Dec 2016 14:54:44 +0000 (16:54 +0200)]
Bug 17783: Prevent crash when providing an undefined value

When calling the proposed version of get_effective_issuing_rule with undefined
parameter values, a following crash occurs:

SQL::Abstract::puke(): [SQL::Abstract::__ANON__] Fatal: SQL::Abstract before v1.75
used to generate incorrect SQL when the -IN operator was given an undef-containing
list: !!!AUDIT YOUR CODE AND DATA!!! (the upcoming Data::Query-based version of
SQL::Abstract will emit the logically correct SQL instead of raising this
exception) at /home/ubuntu/kohaclone/Koha/Objects.pm line 182

This patch adds a test to cover this problem and fixes the issue.

To test:
1. Run t/db_dependent/Koha/IsssuingRules.t

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>
7 years agoBug 17783: Optimize Koha::IssuingRules->get_effective_issuing_rule
Lari Taskula [Thu, 15 Dec 2016 17:31:30 +0000 (19:31 +0200)]
Bug 17783: Optimize Koha::IssuingRules->get_effective_issuing_rule

This patch modifies method get_effective_issuing_rule in Koha::IssuingRules
aiming to optimize the search for matching issuing rule.

Before this patch, in worst case scenario, we have had to make a SELECT query
eight times. This will have a negative impact on performance where-ever we need
to find matching issuing rule multiple times, if the search is not directly
matching an issuing rule on the first query.

This patch makes get_effective_issuing_rule have a stable performance on both
best and worst case, whereas the old method was really fast on the best case
and really slow on the worst case.

However, this patch slightly lowers the performance for best case, where matching
issuing rule is found instantly before (branchcode, categorycode and itemtype all
are specifically defined in issuing rules). For all other cases this patch offers
a performance improvement.

To test:
1. Run t/db_dependent/Koha/IssuingRules.t and compare the results with previous
   tests.

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>
7 years agoBug 17783: Add Koha::Objects->single
Lari Taskula [Thu, 15 Dec 2016 17:28:38 +0000 (19:28 +0200)]
Bug 17783: Add Koha::Objects->single

Returns one and only one object that is part of this set.
Returns undef if there are no objects found.

->single is faster than ->search->next

This is optimal as it will grab the first returned result without instantiating
a cursor.

It is useful for this Bug as we only want to select the top row of found issuing
rules.

To test:
1. Run t/db_dependent/Koha/Objects.t

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>
7 years agoBug 17783: Test to print performance of get_effective_issuing_rule
Lari Taskula [Thu, 15 Dec 2016 16:35:13 +0000 (18:35 +0200)]
Bug 17783: Test to print performance of get_effective_issuing_rule

This test prints the amount of issuing rule matches per second for
1. worst case, when non-existent branchcode, categorycode and itemtype is
   being searched (currently 8 queries)
2. mid case (rule found on 4th query)
3. 2nd best case (rule found on 2nd query)
4. best case, when an issuing rule is defined for exactly those branchcode,
   categorycode and itemtype (currently 1 query)

To test:
1. Run t/db_dependent/Koha/IssuingRules.t
2. Write down the per-second amount to compare with next patch

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>
7 years agoBug 17783: Test to confirm correct effective issuing rule selection
Lari Taskula [Thu, 15 Dec 2016 15:58:39 +0000 (17:58 +0200)]
Bug 17783: Test to confirm correct effective issuing rule selection

This patch adds a test to cover the validity of effective issuing rule selection
in correct order.

To test:
1. Run t/db_dependent/Koha/IssuingRules.t

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>
7 years agoBug 17689: Add the Koha::Checkout->is_overdue method
Jonathan Druart [Fri, 25 Nov 2016 13:16:26 +0000 (14:16 +0100)]
Bug 17689: Add the Koha::Checkout->is_overdue method

This patch adds a new method Koha::Checkout->is_overdue and provide tests
to cover it.
The goal is to behave like GetItemIssues set the 'overdue' flag to
issues.
I don't understand why the existing GetItemIssues truncate dates to
minutes, so I did not recreate this behavior.

Test plan:
  prove t/db_dependent/Koha/Checkouts.t
should return green

Signed-off-by: Mika Smith <mikasmith@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17678: Use Koha::Checkouts instead of Koha::Issues
Jonathan Druart [Tue, 20 Dec 2016 13:45:33 +0000 (14:45 +0100)]
Bug 17678: Use Koha::Checkouts instead of Koha::Issues

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17678: Typo iss in Circulation.t
Marcel de Rooy [Fri, 2 Dec 2016 08:57:50 +0000 (09:57 +0100)]
Bug 17678: Typo iss in Circulation.t

Iss should be is.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17678: C4::Circulation - Replace GetIssues with Koha::Issues
Jonathan Druart [Fri, 25 Nov 2016 10:12:20 +0000 (11:12 +0100)]
Bug 17678: C4::Circulation - Replace GetIssues with Koha::Issues

The C4::Circulation::GetIssues subroutine is only called once and can be
replaced with a call to Koha::Isues->search with a join on items.

Test plan:
- Apply first patch and make sure the tests pass
- Apply second patch and make sure the tests still pass

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17678: Add tests for CanBookBeIssued + AllowMultipleIssuesOnABiblio
Jonathan Druart [Fri, 25 Nov 2016 10:06:25 +0000 (11:06 +0100)]
Bug 17678: Add tests for CanBookBeIssued + AllowMultipleIssuesOnABiblio

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17398: Enhance circulation messages UI
Josef Moravec [Tue, 4 Oct 2016 12:55:51 +0000 (14:55 +0200)]
Bug 17398: Enhance circulation messages UI

Test plan:
1) Apply patch
2) Add same circulation messages, note that both buttons are in bootstrap style and the whole form is a bit cleaner
3) Confirm that adding works as expected
4) Try to delete some of your messages, note the delete link is also button now
5) Confirm that deleting works as expected

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17317: Perfomance Improvement
Lyon3 Team [Fri, 30 Sep 2016 12:50:16 +0000 (14:50 +0200)]
Bug 17317: Perfomance Improvement

- Use of GetItemnumbersForBiblio instead of GetItemsByBiblioitemnumber (thx Jonathan Druart)

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17317: ILSDI: Getavailability method with id_type=bib implementation
Lyon3 Team [Tue, 20 Sep 2016 10:13:23 +0000 (12:13 +0200)]
Bug 17317: ILSDI: Getavailability method with id_type=bib implementation

To test this patch you should try to send a query to you opac formatted
this way :
http://[your-opac-domain-name]/cgi-bin/koha/ilsdi.pl?service=GetAvailability&id=[biblionumber]&id_type=bib

You should get availability status for all the items of the matched
bibliographic record.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17208: Checking if classification source or filing rule already exists before...
Aleisha Amohia [Fri, 2 Dec 2016 04:08:45 +0000 (04:08 +0000)]
Bug 17208: Checking if classification source or filing rule already exists before adding

To test:
1) Go to Admin -> Classification sources
2) Attempt to reproduce bug before applying patch. Notice the message
saying the add was successful, but was not added to the table
3) Apply patch and refresh page (restart plack if necessary)
4) Add new classification source with same code as existing one. Notice
you are now told that the add failed.
5) Confirm adding new classification source with unique code works
6) Add new classification filing rule with same code as existing one.
Notice you are told that add failed.
7) Confirm adding new classification filing rule with unique code works

Sponsored-by: Catalyst IT
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
This is not the way to go, we should use an eval instead.
But since we do not have RaiseError set, it will not work.
This module will need to be moved to Koha::Objects to be implemented
correctly.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15415 [QA Followup] - Make code more readable
Kyle M Hall [Fri, 23 Dec 2016 11:31:00 +0000 (11:31 +0000)]
Bug 15415 [QA Followup] - Make code more readable

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15415: Warn when creating a new print profile
Aleisha Amohia [Fri, 2 Dec 2016 03:04:07 +0000 (03:04 +0000)]
Bug 15415: Warn when creating a new print profile

As per Jonathan's comment in Comment 3, I've put that line of code in an
if statement that will only call the get_attr method if we are editing
an existing profile (therefore the profile id will exist).

To test:
1) Go to Tools -> Patron Card Creator -> New printer profile
2) Notice warn
3) Apply patch
4) Refresh page
5) Confirm warn is gone and page still works as expected

Sponsored-by: Catalyst IT
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17804: Remove some modules from showdiffmarc.pl
Marcel de Rooy [Wed, 21 Dec 2016 13:13:23 +0000 (14:13 +0100)]
Bug 17804: Remove some modules from showdiffmarc.pl

Remove DBI, LibXML, LibXSLT.
Add CGI's -utf8 flag.
Few whitespace changes.

Test plan:
[1] Export an existing record to marcxml.
[2] Edit the file, make some small changes.
[3] Import it again, use a matching rule.
[4] Check the diff on Manage staged. (Here is showdiffmarc.pl)

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>
7 years agoBug 16933 - Update documentation/help file.
Dani Elder [Mon, 10 Oct 2016 13:51:43 +0000 (09:51 -0400)]
Bug 16933 - Update documentation/help file.

Updates help documentation about Alt+W to open renew tab.

To test: Go to circulation help page, open help file and see that new
line mentioning Alt+W.

Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16933 - Alt-Y not working on "Please confirm checkout" dialogs
Tim McMahon [Thu, 8 Sep 2016 16:19:17 +0000 (12:19 -0400)]
Bug 16933 - Alt-Y not working on "Please confirm checkout" dialogs

This patch changes the keyboard shortcut for renew from Alt+y to Alt+w.

To test:
1) Press Alt+y when you get a "Please confirm checkout" dialog.
2) The renew tab is selected instead of confirming the dialog.
3) Apply the patch and refresh your browser to load the change.
4) Repeat step one.
5) Alt+y confirms the checkout, Alt+w selects the renew tab.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16914: Remove unused empty_lines.inc file
Tomas Cohen Arazi [Thu, 22 Dec 2016 15:46:06 +0000 (12:46 -0300)]
Bug 16914: Remove unused empty_lines.inc file

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>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
7 years agoBug 16914: Rely on TT for newlines
Tomas Cohen Arazi [Thu, 22 Dec 2016 15:45:08 +0000 (12:45 -0300)]
Bug 16914: Rely on TT for newlines

The translation scripts have an historical tendency to chomp newlines
and we introduced an empty_line.inc file to force newlines when building
CSV output out of our templates (in item search and late orders).

This patch makes the mentioned templates use TT ability to force newlines
plus some misuses of the 'minus' sign.

Test plan:
- Apply the patch
- Do an item search that returns more than one result
- Export as CSV
=> SUCCESS: The CSV file is correctly formed.
- Install any translation:
  $ sudo koha-shell kohadev
 k$ cd kohaclone/misc/translator
 k$ perl translate install <chosen language>
- Enable <chosen language> (e.g. es-ES)
- Repeat the item search
- Export as CSV
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Have more than one late orders (bummer)
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed.
- Switch language
- Go to late orders
- Choose them
- Export as CSV (in english)
=> SUCCESS: The CSV file is correctly formed in your chosen language.
- Sign off :-D

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>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
7 years agoBug 17764: (bug 17556 follow-up) Fix search for logged out users and lost items
Jonathan Druart [Tue, 13 Dec 2016 12:28:35 +0000 (12:28 +0000)]
Bug 17764: (bug 17556 follow-up) Fix search for logged out users and lost items

If you are non logged-in and you the search result contain lost items,
you will get:
Can't call method "category" on an undefined value at
/home/liz/koha-src/koha/C4/Search.pm line 2091.

This is because bug 17556 assumed that $userenv was not defined when the
user is logged out. Actually it is, with non defined or empty string
values.

Test plan:
Do a search in the opac that would turn up a whole list of results (and
not just that one) with the lost item included.
 => Without this patch you should get an error
 => With this patch applied you should see the search results

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
7 years agoBug 17743: Item search - Fix indexes build on MARC
Jonathan Druart [Thu, 15 Dec 2016 10:47:24 +0000 (10:47 +0000)]
Bug 17743: Item search - Fix indexes build on MARC

Searching items by custom search fields does not work because these
fields are not correctly processed in JS.

In case of custom search field, the parent of the option is not the
select but the optgroup element.

Test plan:
Create a custom search field on 245$c for instance
On the items search form, select this field and launch a search
=> Without this patch, the results will not be filtered and you will get
all your items
=> With this patch applied, the results should be correctly filtered

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>
7 years agoBug 16951: Replace some more
Jonathan Druart [Thu, 15 Dec 2016 12:50:42 +0000 (13:50 +0100)]
Bug 16951: Replace some more

In order to avoid warnings in the logs, $cgi->param should be forced to
scalar context

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>
7 years agoBug 16951: Fix Item search sorting
Jonathan Druart [Thu, 15 Dec 2016 12:45:22 +0000 (13:45 +0100)]
Bug 16951: Fix Item search sorting

Caused by
  commit ac5a1bfececb5400a77f0ebad90181f5215d5a85
    Bug 16154: CGI->multi_param - Manual changes

The change was wrong, we wanted to retrieve a scalar (the string), not
an array.
We want to retrieve a string with the different column' names, not an
array of 1 element.

Test plan:
Launch an item search and play with column sort

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>
7 years agoBug 17692: Remove 'CGI::param called in list context' warnings
Jonathan Druart [Tue, 20 Dec 2016 14:29:49 +0000 (14:29 +0000)]
Bug 17692: Remove 'CGI::param called in list context' warnings

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17692 - Can't add library EAN under Plack
Kyle M Hall [Thu, 15 Dec 2016 11:10:38 +0000 (11:10 +0000)]
Bug 17692 - Can't add library EAN under Plack

When adding a library ean under plack, there is an "internal server
error" when running Plack. Tested with Koha 16.11 release.

Test Plan:
1) Apply this patch
2) Enable Plack
3) Add, Edit and Delete a Library EAN

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17796: Move Koha::OldIssue[s] to Koha::Checkout[s]
Jonathan Druart [Tue, 20 Dec 2016 13:28:00 +0000 (14:28 +0100)]
Bug 17796: Move Koha::OldIssue[s] to Koha::Checkout[s]

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17796: Replace Koha::Issue[s] with Koha::Checkout[s]
Jonathan Druart [Tue, 20 Dec 2016 12:57:00 +0000 (13:57 +0100)]
Bug 17796: Replace Koha::Issue[s] with Koha::Checkout[s]

Koha::Issues and Koha::Checkouts have been added to the codebase to
represent the same thing.

In ODLIS the word Issue is never used in the sense we use it. Another
problem with Issue is it has so many meaning in English (such as
problem/bug)
The word Checkout *is* in ODLIS, closer to what we use:
http://www.abc-clio.com/ODLIS/odlis_c.aspx#checkoutslip

Test plan:
  git grep Koha::Issue
should not return any occurrences and the tests must still pass

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17790: Fix js error on undefined autocomplete(...).data(...)
Marcel de Rooy [Mon, 19 Dec 2016 11:55:59 +0000 (12:55 +0100)]
Bug 17790: Fix js error on undefined autocomplete(...).data(...)

Bug 17418 moved some code to js_includes.inc.
But if #findborrower does not exist, you cannot define _renderItem.
Trivial fix.

Test plan:
[1] Find a page where this include is used and #findborrowers is absent.
    Like about.pl
[2] Without this patch, you will have a js error in the js console.
[3] With this patch, you should no longer have it.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17631: Koha::Biblio - Remove GetHolds
Jonathan Druart [Tue, 15 Nov 2016 14:01:41 +0000 (14:01 +0000)]
Bug 17631: Koha::Biblio - Remove GetHolds

C4::Biblio::GetHolds can be replaced with Koha::Biblio->holds->count

Test plan:
Create an order and place a hold on the biblio you have ordered.
On the basket view, you should not be able to Cancel the order and/or
delete the record
Receive the order, on the parcel page you should get the same behavior.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17630: Add the Koha::Biblio->holds method
Jonathan Druart [Tue, 15 Nov 2016 11:37:30 +0000 (11:37 +0000)]
Bug 17630: Add the Koha::Biblio->holds method

This method will be useful to get the current holds placed on a given
bibliographic record.

Test plan:
  prove t/db_dependent/Koha/Biblios.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17586: Move ->get_balance to Koha::Account->balance
Jonathan Druart [Tue, 6 Dec 2016 08:12:57 +0000 (09:12 +0100)]
Bug 17586: Move ->get_balance to Koha::Account->balance

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17586: Add the Koha::Account::Lines->get_balance method
Jonathan Druart [Tue, 8 Nov 2016 13:50:10 +0000 (13:50 +0000)]
Bug 17586: Add the Koha::Account::Lines->get_balance method

Test plan:
  prove t/db_dependent/Accounts.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17585: Replace ->get_account_lines with ->account
Jonathan Druart [Tue, 6 Dec 2016 07:58:34 +0000 (07:58 +0000)]
Bug 17585: Replace ->get_account_lines with ->account

And return a Koha::Account instead of a Koha::Account::Lines

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17585: Add the Koha::Patron->get_account_lines method
Jonathan Druart [Tue, 8 Nov 2016 13:19:39 +0000 (13:19 +0000)]
Bug 17585: Add the Koha::Patron->get_account_lines method

Test plan:
  prove t/db_dependent/Koha/Patrons.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17578 [QA Followup] - Update number of tests
Kyle M Hall [Fri, 16 Dec 2016 13:14:50 +0000 (13:14 +0000)]
Bug 17578 [QA Followup] - Update number of tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBut 17578: (followup) amountoutstanding
Jonathan Druart [Tue, 29 Nov 2016 18:20:13 +0000 (19:20 +0100)]
But 17578: (followup) amountoutstanding

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>
7 years agoBug 17578: GetMemberDetails - Remove GetMemberDetails
Jonathan Druart [Tue, 8 Nov 2016 10:03:54 +0000 (11:03 +0100)]
Bug 17578: GetMemberDetails - Remove GetMemberDetails

All the values different from the ones GetMember returned has been
managed outside of GetMemberDetails.
It looks safe to replace all the occurrences of GetMemberDetails with
GetMember.

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>
7 years agoBug 17578: GetMemberDetails - Remove enrolmentperiod
Jonathan Druart [Tue, 8 Nov 2016 09:47:53 +0000 (09:47 +0000)]
Bug 17578: GetMemberDetails - Remove enrolmentperiod

This value is not used anywhere

Test plan:
  git grep enrolmentperiod| grep -v installer| grep -v translator|vim -

should show you that I am right

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>
7 years agoBug 17578: GetMemberDetails - Remove reservefee
Jonathan Druart [Tue, 8 Nov 2016 09:46:15 +0000 (09:46 +0000)]
Bug 17578: GetMemberDetails - Remove reservefee

Same as other patches, reservefee is only used in opac-reserve.pl

Test plan;
Set reserve fee for a patron category
Place a hold at the OPAC with one of these patrons.
You must get a message about the reserve fee.

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>
7 years agoBug 17578: GetMemberDetails - Remove is_expired
Jonathan Druart [Mon, 7 Nov 2016 17:14:49 +0000 (18:14 +0100)]
Bug 17578: GetMemberDetails - Remove is_expired

The is_expired value is used in 2 places, let's use
Koha::Patron->is_expired instead.

Test plan:
Depending on the different value of BlockExpiredPatronOpacActions for
the patron category, a patron must be blocked if he has expired.
Confirm that behavior from opac-renew and opac-reserve scripts

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>
7 years agoBug 17578: GetMemberDetails - Remove flags
Jonathan Druart [Mon, 7 Nov 2016 16:52:54 +0000 (16:52 +0000)]
Bug 17578: GetMemberDetails - Remove flags

Same as authflags, a flags key is set containing all the patron flags.
It is only used in a few places and it's better to call
C4::Members::patronflags when we need it.

Test plan:
Look at the diff and confirm that the change make sense
Use git grep to confirm we do not use the flags somewhere else.

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>
7 years agoBug 17578: GetMemberDetails - Remove amountoutstanding
Jonathan Druart [Mon, 7 Nov 2016 16:16:04 +0000 (16:16 +0000)]
Bug 17578: GetMemberDetails - Remove amountoutstanding

The amountoutstanding value set by GetMemberDetails was only used in a
few places. In that case it makes sense to only retrieve it when needed.

Test plan:
1/ Add fines to a patron, on the OPAC patron info page, you should see a
"Fines" tab
2/ Add credit to a patron, you should see the credit displayed
3/ Set the pref maxoutstanding to 3
4/ Add a fine of 4 to a patron
5/ Try to place an hold for this patron
=> You should get a "too much oweing" message

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>
7 years agoBug 17578: GetMemberDetails - Remove authflags - 2
Jonathan Druart [Mon, 7 Nov 2016 16:01:03 +0000 (17:01 +0100)]
Bug 17578: GetMemberDetails - Remove authflags - 2

This script is not used from the Koha codebase and does not seem very
useful.
We could rewrite it if needed later (ie. if someone complains I will
rewrite it).

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>
7 years agoBug 17578: GetMemberDetails - Remove authflags - 1
Jonathan Druart [Mon, 7 Nov 2016 16:02:55 +0000 (17:02 +0100)]
Bug 17578: GetMemberDetails - Remove authflags - 1

GetMemberDetails create a authflags key, but this key is only used from
2 different places.
One is a very simple script, which does not seem very usefull
C4/SIP/interactive_members_dump.pl. I propose to simply remove it.
The other one is the member-flags.pl script. What is done in this one is
a bit weird since we a doing twice the same query (it was not highlighted
before this patch). We will need to fix that later.
At the moment the goal it to remove the GetMemberDetails subroutine
without introducing any regressions (and so without adding big changes)

Test plan:
Select/unselect permissions for a patron, save and edit again.
The behavior of the permission checkboxes should be ok

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>
7 years agoBug 17578: GetMemberDetails - Remove BlockExpiredPatronOpacActions
Jonathan Druart [Mon, 7 Nov 2016 15:53:25 +0000 (15:53 +0000)]
Bug 17578: GetMemberDetails - Remove BlockExpiredPatronOpacActions

The correct way to get the value of BlockExpiredPatronOpacActions from a
patron object is to get the patron category then call the
effective_BlockExpiredPatronOpacActions:
  $patron->category->effective_BlockExpiredPatronOpacActions

So this patch applies this change and remove this value from the
GetMemberDetails subroutine.

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>
7 years agoBug 17568: Add the Koha::Patron->library method
Jonathan Druart [Mon, 7 Nov 2016 14:24:54 +0000 (14:24 +0000)]
Bug 17568: Add the Koha::Patron->library method

This method will be convenient when moving code to Koha::Patrons

Test plan:
  prove t/db_dependent/Koha/Patrons.t
should return green

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>
7 years agoBug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge)
Jonathan Druart [Fri, 4 Nov 2016 16:21:03 +0000 (16:21 +0000)]
Bug 17557: Koha::Patrons - Move GetAge to ->set_age (and remove SetAge)

As said in the previous commit, I considered SetAge as unnecessary and
removed it.

Test plan:
1/ Edit a patron using the different 'Edit' links
2/ Play with the patron category limited to age ranges, and date of
birth
3/ You should get the expected warning if the date of birth is inside
the patron category date range.

To finish:
  prove t/Circulation/AgeRestrictionMarkers.t t/db_dependent/Reserves.t \
        t/db_dependent/Koha/Patrons.t t/db_dependent/Members.t
should return green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17557: Revised patron age calculation tests
Jonathan Druart [Fri, 4 Nov 2016 16:18:00 +0000 (16:18 +0000)]
Bug 17557: Revised patron age calculation tests

The SetAge and GetAge test coverage are excessive.
First the SetAge subroutine was only created for testing purpose.
The goal of GetAge is quite simple and it seems quite easy to provide
corect test coverage using DateTime->add using negative numbers.

Edit: rebased so it applies

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
7 years agoBug 16203: Convert item plugins to new style (see bug 10480)
Marcel de Rooy [Tue, 5 Apr 2016 12:51:43 +0000 (14:51 +0200)]
Bug 16203: Convert item plugins to new style (see bug 10480)

Converts item plugins to new style (with builder and launcher).
See also bugs 10480 and 13437.

The following plugins have been adjusted:
barcode_manual.pl
barcode.pl
callnumber-KU.pl
callnumber.pl
cn_browser.pl (Added license statement too)
dateaccessioned.pl
macles.pl
stocknumberam123.pl
stocknumberAV.pl
stocknumber.pl

Test plan:
Connect the plugin to an item field.
Verify that the plugin still works.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested if all plugins compile okay.
Ran most of them thru FrameworkPlugin.t.
Tested them in the item editor.

Note: the form for macles.pl comes up, further hard to test.

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>
7 years agoBug 17472: Borrower Previously Checked Out: Display title
Josef Moravec [Mon, 7 Nov 2016 09:32:54 +0000 (09:32 +0000)]
Bug 17472: Borrower Previously Checked Out: Display title

Test plan:
0) apply the patch
1) turn on checking previously checkouts - syspref CheckPrevCheckout
2) checkout an item to borrower
3) check the item in and check it out again, you should see the
dialog, with information that the current item was previously checked
out to this patron. With this patch, the information includes title and
author of the item

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17418 - Move staff client home page JavaScript to the footer
Owen Leonard [Mon, 10 Oct 2016 09:52:51 +0000 (05:52 -0400)]
Bug 17418 - Move staff client home page JavaScript to the footer

This patch alters the header and footer include files so that JavaScript
can be included in either one or the other. As a proof of concept, the
staff client home page is updated to include JS in the footer instead
of the header.

The processing of JavaScript included on individual pages can now be
similar to how it is done in the OPAC. A block is created with the
page's JavaScript which is then processed in js_includes.inc in the
correct order, after other required js assets.

On pages which have been modified to allow JavaScript to be moved to the
footer you must add a variable to the template: [% SET footerjs = 1 %].
Eventually all staff client templates should be modified so that setting
a flag is not required.

"[% MACRO jsinclude BLOCK %]" is used instead of "[% BLOCK %]" and "[%
PROCESS %]" because MACRO allows the template directives to be
processed correctly when included by intranet-bottom.inc.

To test, apply the patch and view the staff client home page.

- Confirm that you get a confirmation when deleting a news item from the
  home page.
- Enable the CircAutocompl system preference and test that patron
  autocomplete works from the "Check out" tab from the staff home page
  and from other pages where the "Check out" tab is present.
- Test that JavaScript is working correctly on other pages like
  Circulation, Preferences, etc.

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>